From ba491c7ee08975346ae749fc3912b581b78421c2 Mon Sep 17 00:00:00 2001 From: mace Date: Mon, 6 Jul 2020 18:13:36 +0200 Subject: [PATCH] Add 0x check for Address, Bytes, and Bytes32 in GraphQl requests (#1057) Produce error response if any of Address, Bytes or Bytes32 values inside GraphQl requests are not prefixed with 0x as defined by the spec. Fix old tests that used values that are not prefixed with 0x and add additional tests to confirm Address, Bytes and Bytes32 values will be rejected if not prefixed with 0x. Signed-off-by: Mak Muftic --- .../api/graphql/internal/Scalars.java | 22 ++++++++++++++++--- .../jsonrpc/internal/results/Quantity.java | 10 +++++++++ .../api/graphql/EthGraphQLHttpBySpecTest.java | 3 +++ .../ethereum/api/graphql/eth_call_Block8.json | 2 +- .../eth_call_Block8_invalidHexBytesData.json | 21 ++++++++++++++++++ .../api/graphql/eth_call_BlockLatest.json | 2 +- .../eth_getBalance_invalidHexAddress.json | 20 +++++++++++++++++ ...getBlock_byHash_InvalidHexBytes32Hash.json | 20 +++++++++++++++++ .../ethereum/api/graphql/graphql_pending.json | 2 +- 9 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_call_Block8_invalidHexBytesData.json create mode 100644 ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_getBalance_invalidHexAddress.json create mode 100644 ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_getBlock_byHash_InvalidHexBytes32Hash.json diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/Scalars.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/Scalars.java index 4b646b9f55..4a98d9a49a 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/Scalars.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/Scalars.java @@ -14,6 +14,7 @@ */ package org.hyperledger.besu.ethereum.api.graphql.internal; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.Quantity; import org.hyperledger.besu.ethereum.core.Address; import org.hyperledger.besu.ethereum.core.Hash; @@ -55,8 +56,13 @@ public class Scalars { if (!(input instanceof StringValue)) { throw new CoercingParseLiteralException("Value is not any Address : '" + input + "'"); } + String inputValue = ((StringValue) input).getValue(); + if (!Quantity.isValid(inputValue)) { + throw new CoercingParseLiteralException( + "Address value '" + inputValue + "' is not prefixed with 0x"); + } try { - return Address.fromHexStringStrict(((StringValue) input).getValue()); + return Address.fromHexStringStrict(inputValue); } catch (final IllegalArgumentException e) { throw new CoercingParseLiteralException("Value is not any Address : '" + input + "'"); } @@ -121,8 +127,13 @@ public class Scalars { if (!(input instanceof StringValue)) { throw new CoercingParseLiteralException("Value is not any Bytes : '" + input + "'"); } + String inputValue = ((StringValue) input).getValue(); + if (!Quantity.isValid(inputValue)) { + throw new CoercingParseLiteralException( + "Bytes value '" + inputValue + "' is not prefixed with 0x"); + } try { - return Bytes.fromHexStringLenient(((StringValue) input).getValue()); + return Bytes.fromHexStringLenient(inputValue); } catch (final IllegalArgumentException e) { throw new CoercingParseLiteralException("Value is not any Bytes : '" + input + "'"); } @@ -156,8 +167,13 @@ public class Scalars { if (!(input instanceof StringValue)) { throw new CoercingParseLiteralException("Value is not any Bytes32 : '" + input + "'"); } + String inputValue = ((StringValue) input).getValue(); + if (!Quantity.isValid(inputValue)) { + throw new CoercingParseLiteralException( + "Bytes32 value '" + inputValue + "' is not prefixed with 0x"); + } try { - return Bytes32.fromHexStringLenient(((StringValue) input).getValue()); + return Bytes32.fromHexStringLenient(inputValue); } catch (final IllegalArgumentException e) { throw new CoercingParseLiteralException("Value is not any Bytes32 : '" + input + "'"); } diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/Quantity.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/Quantity.java index 21ee63ed67..f2e7617813 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/Quantity.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/Quantity.java @@ -77,6 +77,16 @@ public class Quantity { return String.format("%s%s%s", HEX_PREFIX, zeroPadding, formatted); } + /** + * Checks if value is prefixed with '0x' + * + * @param value that is checked + * @return true if value is prefixed with '0x', false otherwise + */ + public static boolean isValid(final String value) { + return value.startsWith(HEX_PREFIX); + } + private static String uint256ToHex(final UInt256Value value) { return value == null ? null : formatMinimalValue(value.toMinimalBytes().toShortHexString()); } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/EthGraphQLHttpBySpecTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/EthGraphQLHttpBySpecTest.java index 4f4f9088e0..04bcd51206 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/EthGraphQLHttpBySpecTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/EthGraphQLHttpBySpecTest.java @@ -47,6 +47,7 @@ public class EthGraphQLHttpBySpecTest extends AbstractEthGraphQLHttpServiceTest specs.add("eth_blockNumber"); specs.add("eth_call_Block8"); + specs.add("eth_call_Block8_invalidHexBytesData"); specs.add("eth_call_BlockLatest"); specs.add("eth_estimateGas_transfer"); @@ -58,11 +59,13 @@ public class EthGraphQLHttpBySpecTest extends AbstractEthGraphQLHttpServiceTest specs.add("eth_getBalance_0x19"); specs.add("eth_getBalance_invalidAccountBlockNumber"); specs.add("eth_getBalance_invalidAccountLatest"); + specs.add("eth_getBalance_invalidHexAddress"); specs.add("eth_getBalance_latest"); specs.add("eth_getBalance_toobig_bn"); specs.add("eth_getBalance_without_addr"); specs.add("eth_getBlock_byHash"); + specs.add("eth_getBlock_byHash_InvalidHexBytes32Hash"); specs.add("eth_getBlock_byHashInvalid"); specs.add("eth_getBlock_byNumber"); specs.add("eth_getBlock_byNumberInvalid"); diff --git a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_call_Block8.json b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_call_Block8.json index c2d0014b30..26a2eb4a45 100644 --- a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_call_Block8.json +++ b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_call_Block8.json @@ -1,5 +1,5 @@ { - "request": "{block(number :\"0x8\") {number call (data : {from : \"a94f5374fce5edbc8e2a8697c15331677e6ebf0b\", to: \"0x6295ee1b4f6dd65047762f924ecd367c17eabf8f\", data :\"0x12a7b914\"}){data status}}}" + "request": "{block(number :\"0x8\") {number call (data : {from : \"0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b\", to: \"0x6295ee1b4f6dd65047762f924ecd367c17eabf8f\", data :\"0x12a7b914\"}){data status}}}" , "response":{ "data" : { diff --git a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_call_Block8_invalidHexBytesData.json b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_call_Block8_invalidHexBytesData.json new file mode 100644 index 0000000000..2d876b4270 --- /dev/null +++ b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_call_Block8_invalidHexBytesData.json @@ -0,0 +1,21 @@ +{ + "request": "{block(number :\"0x8\") {number call (data : {from : \"0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b\", to: \"0x6295ee1b4f6dd65047762f924ecd367c17eabf8f\", data :\"12a7b914\"}){data status}}}" + , + "response": { + "errors": [ + { + "message": "Validation error of type WrongType: argument 'data.data' with value 'StringValue{value='12a7b914'}' is not a valid 'Bytes' - Bytes value '12a7b914' is not prefixed with 0x @ 'block/call'", + "locations": [ + { + "line": 1, + "column": 37 + } + ], + "extensions": { + "classification": "ValidationError" + } + } + ] + }, + "statusCode": 400 +} \ No newline at end of file diff --git a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_call_BlockLatest.json b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_call_BlockLatest.json index 129811c38f..7478c29a74 100644 --- a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_call_BlockLatest.json +++ b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_call_BlockLatest.json @@ -1,5 +1,5 @@ { - "request": "{block {number call (data : {from : \"a94f5374fce5edbc8e2a8697c15331677e6ebf0b\", to: \"0x6295ee1b4f6dd65047762f924ecd367c17eabf8f\", data :\"0x12a7b914\"}){data status}}}" + "request": "{block {number call (data : {from : \"0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b\", to: \"0x6295ee1b4f6dd65047762f924ecd367c17eabf8f\", data :\"0x12a7b914\"}){data status}}}" , "response":{ "data" : { diff --git a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_getBalance_invalidHexAddress.json b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_getBalance_invalidHexAddress.json new file mode 100644 index 0000000000..ba027020c7 --- /dev/null +++ b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_getBalance_invalidHexAddress.json @@ -0,0 +1,20 @@ +{ + "request": "{account(blockNumber:\"0x19\", address: \"6295ee1b4f6dd65047762f924ecd367c17eabf8f\") { balance } }", + "response": { + "errors": [ + { + "message": "Validation error of type WrongType: argument 'address' with value 'StringValue{value='6295ee1b4f6dd65047762f924ecd367c17eabf8f'}' is not a valid 'Address' - Address value '6295ee1b4f6dd65047762f924ecd367c17eabf8f' is not prefixed with 0x @ 'account'", + "locations": [ + { + "line": 1, + "column": 30 + } + ], + "extensions": { + "classification": "ValidationError" + } + } + ] + }, + "statusCode": 400 +} diff --git a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_getBlock_byHash_InvalidHexBytes32Hash.json b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_getBlock_byHash_InvalidHexBytes32Hash.json new file mode 100644 index 0000000000..792775e910 --- /dev/null +++ b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/eth_getBlock_byHash_InvalidHexBytes32Hash.json @@ -0,0 +1,20 @@ +{ + "request": "{block (hash : \"c8df1f061abb4d0c107b2b1a794ade8780b3120e681f723fe55a7be586d95ba6\") {number } }", + "response": { + "errors": [ + { + "message": "Validation error of type WrongType: argument 'hash' with value 'StringValue{value='c8df1f061abb4d0c107b2b1a794ade8780b3120e681f723fe55a7be586d95ba6'}' is not a valid 'Bytes32' - Bytes32 value 'c8df1f061abb4d0c107b2b1a794ade8780b3120e681f723fe55a7be586d95ba6' is not prefixed with 0x @ 'block'", + "locations": [ + { + "line": 1, + "column": 9 + } + ], + "extensions": { + "classification": "ValidationError" + } + } + ] + }, + "statusCode": 400 +} \ No newline at end of file diff --git a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_pending.json b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_pending.json index 0acb42b455..46cad8c1c7 100644 --- a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_pending.json +++ b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_pending.json @@ -1,6 +1,6 @@ { "request": - "{ pending { transactionCount transactions { nonce gas } account(address:\"0x6295ee1b4f6dd65047762f924ecd367c17eabf8f\") { balance} estimateGas(data:{}) call (data : {from : \"a94f5374fce5edbc8e2a8697c15331677e6ebf0b\", to: \"0x6295ee1b4f6dd65047762f924ecd367c17eabf8f\", data :\"0x12a7b914\"}){data status}} }", + "{ pending { transactionCount transactions { nonce gas } account(address:\"0x6295ee1b4f6dd65047762f924ecd367c17eabf8f\") { balance} estimateGas(data:{}) call (data : {from : \"0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b\", to: \"0x6295ee1b4f6dd65047762f924ecd367c17eabf8f\", data :\"0x12a7b914\"}){data status}} }", "response": { "data": {