You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hive.apache.org by "Marcono1234 (Jira)" <ji...@apache.org> on 2021/03/27 16:50:00 UTC
[jira] [Updated] (HIVE-24952) RcpServer.createSecret() creates
malformed hex strings
[ https://issues.apache.org/jira/browse/HIVE-24952?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Marcono1234 updated HIVE-24952:
-------------------------------
Description:
The method {{org.apache.hive.spark.client.rpc.RcpServer.createSecret()}} creates malformed hex strings:
{code}
public String createSecret() {
byte[] secret = new byte[config.getSecretBits() / 8];
RND.nextBytes(secret);
StringBuilder sb = new StringBuilder();
for (byte b : secret) {
if (b < 10) {
sb.append("0");
}
sb.append(Integer.toHexString(b));
}
return sb.toString();
}
{code}
There are three flaws with this:
- It uses the signed byte value, so for {{(byte) 0xFF}}, {{Integer.toHexString(...)}} will be {{ffffffff}}.
- The {{b < 10}} check is incorrect; I assume it should cover the case the {{toHexString}} only has one hex char as result, however that happens for {{b < 16}}.
- The check {{b <}} compares the signed value, so every value < 0 will also erroneously get a leading {{0}}.
For example, if {{secret}} was randomly generated as {{\{10, -1\}}}, then the result would be:
- Expected: 0aff
- Actual: a0ffffffff
The correct implementation would be:
{code}
public String createSecret() {
byte[] secret = new byte[config.getSecretBits() / 8];
RND.nextBytes(secret);
StringBuilder sb = new StringBuilder();
for (byte b : secret) {
int unsignedB = b & 0xFF;
if (unsignedB < 16) {
sb.append('0');
}
sb.append(Integer.toHexString(unsignedB));
}
return sb.toString();
}
{code}
If this behavior is intended (e.g. because a different project relies on this broken hex string format), then it would at least be good to add a comment explaining this.
was:
The method {{org.apache.hive.spark.client.rpc.RcpServer.createSecret()}} creates malformed hex strings:
{code}
public String createSecret() {
byte[] secret = new byte[config.getSecretBits() / 8];
RND.nextBytes(secret);
StringBuilder sb = new StringBuilder();
for (byte b : secret) {
if (b < 10) {
sb.append("0");
}
sb.append(Integer.toHexString(b));
}
return sb.toString();
}
{code}
There are three flaws with this:
- It uses the signed byte value, so for {{(byte) 0xFF}}, {{Integer.toHexString(...)}} will be {{ffffffff}}.
- The {{b < 10}} check is incorrect; I assume it should cover the case the {{toHexString}} only has one hex char as result, however that happens for {{b < 16}}.
- The check {{b < }} compares the signed value, so every value < 0 will also erroneously get a leading {{0}}.
For example, if {{secret}} was randomly generated as {{\{10, -1\}}}, then the result would be:
- Expected: 0aff
- Actual: a0ffffffff
The correct implementation would be:
{code}
public String createSecret() {
byte[] secret = new byte[config.getSecretBits() / 8];
RND.nextBytes(secret);
StringBuilder sb = new StringBuilder();
for (byte b : secret) {
int unsignedB = b & 0xFF;
if (unsignedB < 16) {
sb.append('0');
}
sb.append(Integer.toHexString(unsignedB));
}
return sb.toString();
}
{code}
If this behavior is intended (e.g. because a different project relies on this broken hex string format), then it would at least be good to add a comment explaining this.
> RcpServer.createSecret() creates malformed hex strings
> ------------------------------------------------------
>
> Key: HIVE-24952
> URL: https://issues.apache.org/jira/browse/HIVE-24952
> Project: Hive
> Issue Type: Bug
> Affects Versions: 2.4.0
> Reporter: Marcono1234
> Priority: Major
>
> The method {{org.apache.hive.spark.client.rpc.RcpServer.createSecret()}} creates malformed hex strings:
> {code}
> public String createSecret() {
> byte[] secret = new byte[config.getSecretBits() / 8];
> RND.nextBytes(secret);
> StringBuilder sb = new StringBuilder();
> for (byte b : secret) {
> if (b < 10) {
> sb.append("0");
> }
> sb.append(Integer.toHexString(b));
> }
> return sb.toString();
> }
> {code}
> There are three flaws with this:
> - It uses the signed byte value, so for {{(byte) 0xFF}}, {{Integer.toHexString(...)}} will be {{ffffffff}}.
> - The {{b < 10}} check is incorrect; I assume it should cover the case the {{toHexString}} only has one hex char as result, however that happens for {{b < 16}}.
> - The check {{b <}} compares the signed value, so every value < 0 will also erroneously get a leading {{0}}.
> For example, if {{secret}} was randomly generated as {{\{10, -1\}}}, then the result would be:
> - Expected: 0aff
> - Actual: a0ffffffff
> The correct implementation would be:
> {code}
> public String createSecret() {
> byte[] secret = new byte[config.getSecretBits() / 8];
> RND.nextBytes(secret);
> StringBuilder sb = new StringBuilder();
> for (byte b : secret) {
> int unsignedB = b & 0xFF;
> if (unsignedB < 16) {
> sb.append('0');
> }
> sb.append(Integer.toHexString(unsignedB));
> }
> return sb.toString();
> }
> {code}
> If this behavior is intended (e.g. because a different project relies on this broken hex string format), then it would at least be good to add a comment explaining this.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)