-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade libthrift to 0.14.1 due CVE-2020-13949 #24462
Upgrade libthrift to 0.14.1 due CVE-2020-13949 #24462
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rebase.
Also, is there a plan to take a look at accumulo to see if thrift can be upgraded for that as well?
fd7554f
to
5b65c96
Compare
The community seems to have little interest in maintaining Accumulo, see #15837 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@denodo-research-labs thanks for the fix! LGTM, just some nits.
...c/main/java/com/facebook/presto/hive/authentication/KerberosHiveMetastoreAuthentication.java
Outdated
Show resolved
Hide resolved
...c/main/java/com/facebook/presto/hive/authentication/KerberosHiveMetastoreAuthentication.java
Outdated
Show resolved
Hide resolved
5b65c96
to
9e30d0a
Compare
New release note guidelines as of last week: PR #24354 automatically adds links to this PR to the release notes. Please remove the manual PR link in the following format from the release note entries for this PR.
I have updated the Release Notes Guidelines to remove the examples of manually adding the PR link. |
9e30d0a
to
2799654
Compare
@Override | ||
public void updateKnownMessageSize(long size) | ||
throws TTransportException | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throw new UnsupportedOperationException("Not implemented");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does transport not have these methods? (transport.updateKnownMessageSize)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or just leave a noop comment if its noop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@Override | ||
public void checkReadBytesAvailable(long numBytes) | ||
throws TTransportException | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throw new UnsupportedOperationException("Not implemented");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added noop comment
2799654
to
ca32cb4
Compare
Description
Fix CVE-2018-1320, CVE-2019-0205, CVE-2019-0210 and CVE-2020-13949.
The version chosen is 0.14.1, although more recent versions exist, because it does not have CRITICAL and HIGH severity CVEs and has been tested in several scenarios with different versions of Hive Metastore.
Fix for #18026
Motivation and Context
The PR upgrades libthrift version to address known CVEs.
Exception made for Accumulo connector, since the version of Accumulo used is not compatible with newer versions of libthrift.
Impact
Dependency org.apache.thrift:libthrift:0.9.3 has four HIGH vulnerabilities:
https://nvd.nist.gov/vuln/detail/CVE-2018-1320
https://nvd.nist.gov/vuln/detail/CVE-2019-0205
https://nvd.nist.gov/vuln/detail/CVE-2019-0210
https://nvd.nist.gov/vuln/detail/CVE-2020-13949
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.