Skip to content
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

Json RPC method parameter validation #2097

Merged
merged 59 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
af96058
Updating BlockHash related methods having validated parameters. Added…
asoto-iov Aug 9, 2023
22cc178
Updated deserializers parent class and updated new eth_getTransaction…
asoto-iov Aug 10, 2023
f1baf19
fix hex input validation
casiojapi Aug 15, 2023
d45a14f
fix unit test exception message
casiojapi Aug 15, 2023
1722017
fix sonarqube warning
casiojapi Aug 16, 2023
50df9f9
add serialVersionUID to deserializer
casiojapi Aug 17, 2023
bc6baa5
Add deserializers for CallArguments, RawData, Address and BlockIdenti…
rmoreliovlabs Aug 26, 2023
ba11d0f
Fix sonarcloud issues
rmoreliovlabs Aug 28, 2023
e9f27a0
Refactor deserializers
rmoreliovlabs Aug 28, 2023
9e04a89
Add BlockRef and BlockRefParam classes
rmoreliovlabs Aug 30, 2023
63e7dd6
Refactor BlockRefParam
rmoreliovlabs Aug 31, 2023
85c2fe0
Adding new Parameter classes
asoto-iov Aug 30, 2023
acbde9e
Refactor BlockRefParam deserialize to check for the param's type
rmoreliovlabs Aug 31, 2023
8c5affe
Updating eth_newFilter method with FilterRequestParam
asoto-iov Aug 31, 2023
27cc17d
Updating eth_getLogs method with FilterRequestParam
asoto-iov Aug 31, 2023
7a9a5d4
Adding param validation to eth_unistallFilter, eth_getFilterChanges a…
asoto-iov Aug 31, 2023
95d61d6
Merge pull request #2117 from rsksmart/json_rpc_param_validation_pt1
Vovchyk Sep 4, 2023
eda346b
Refactor CallArgumentsParam and add HexNumberParam
rmoreliovlabs Sep 4, 2023
5c00e1e
Fix SonarCloud issues
rmoreliovlabs Sep 4, 2023
a0f9f5a
Code cleaning
rmoreliovlabs Sep 5, 2023
7bfa8ee
Add HexKeyParam and HexDurationParam classes
rmoreliovlabs Sep 7, 2023
e970d38
Fix sonar issues
rmoreliovlabs Sep 7, 2023
207d69a
Remove Serializable implementation from HexStringParam child classes
rmoreliovlabs Sep 7, 2023
8cc47ef
Merge branch 'master' into json_rpc_param_validation
Vovchyk Sep 8, 2023
9104c17
Fix failing tests
rmoreliovlabs Sep 8, 2023
e6f9cc7
Implement deserializers
rmoreliovlabs Aug 31, 2023
5963770
Refactor CallArgumentsParam implementation
rmoreliovlabs Sep 5, 2023
d1f32b5
Make sendTransaction method synchronized
rmoreliovlabs Sep 6, 2023
fddec39
Fix code smells
rmoreliovlabs Sep 6, 2023
d0fe948
Suppress Sonar warnings
rmoreliovlabs Sep 6, 2023
b3ed700
Fix failing tests on EthModuleTest
rmoreliovlabs Sep 8, 2023
f36a9fc
Merge pull request #2121 from rsksmart/implement_json_param_validation
Vovchyk Sep 8, 2023
5131f49
Update methods argument classes
rmoreliovlabs Sep 11, 2023
4ad802d
Merge pull request #2130 from rsksmart/json_rpc_param_validation_pt3
Vovchyk Sep 12, 2023
aefe3d7
Remove unused methods
rmoreliovlabs Sep 12, 2023
c717e62
Update methods arguments types
rmoreliovlabs Sep 14, 2023
66589f9
Fix failing tests
rmoreliovlabs Sep 14, 2023
35df451
Merge pull request #2132 from rsksmart/json_rpc_param_validation_pt5
Vovchyk Sep 15, 2023
59ebddf
Merge remote-tracking branch 'origin/master' into json_rpc_param_vali…
Vovchyk Sep 15, 2023
50412d2
Addressing comments
rmoreliovlabs Sep 18, 2023
d770ff7
Fix failing tests
rmoreliovlabs Sep 18, 2023
016b547
Move null check from HexKeyParam to the importRawKey method
rmoreliovlabs Sep 19, 2023
bdc6dd1
Check null value in newAccount method
asoto-iov Sep 20, 2023
82079f9
Merge pull request #2136 from rsksmart/check_null_parameter_for_new_a…
Vovchyk Sep 20, 2023
aabccdd
Revert changes on EthModuleWallet sign method
rmoreliovlabs Sep 20, 2023
61c0687
Remove unused imports
rmoreliovlabs Sep 20, 2023
0dd8a3e
Add (C) header to new files
rmoreliovlabs Sep 21, 2023
5ca790a
Update invalid hex error messages
rmoreliovlabs Sep 21, 2023
8d27c79
Send error object to invalidParamError for logging
rmoreliovlabs Sep 21, 2023
24fe940
Adding support for null topics
asoto-iov Sep 21, 2023
73909be
minor code improvement
asoto-iov Sep 22, 2023
f05a209
Merge pull request #2139 from rsksmart/support_null_topics_in_FilterR…
Vovchyk Sep 22, 2023
66549ed
Merge remote-tracking branch 'origin/master' into json_rpc_param_vali…
Vovchyk Sep 22, 2023
55b42e8
Fixed tests after the merge from master
Vovchyk Sep 22, 2023
3b3bcc6
Added proper handling of number parsing
Vovchyk Sep 22, 2023
5d0a327
Added check for empty string in HexIndexParamTest
Vovchyk Sep 22, 2023
4e64426
Merge pull request #2140 from rsksmart/hex-num-parsing
Vovchyk Sep 22, 2023
59173f6
Merge branch 'master' into json_rpc_param_validation
Vovchyk Oct 3, 2023
bdc22a1
Merge branch 'master' into json_rpc_param_validation
Vovchyk Oct 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions rskj-core/src/main/java/co/rsk/rpc/Web3EthModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
import org.ethereum.rpc.dto.CompilationResultDTO;
import org.ethereum.rpc.dto.TransactionReceiptDTO;
import org.ethereum.rpc.dto.TransactionResultDTO;
import org.ethereum.rpc.parameters.BlockHashParam;
import org.ethereum.rpc.parameters.HexIndexParam;
import org.ethereum.rpc.parameters.TxHashParam;

import java.math.BigInteger;
import java.util.Map;
Expand Down Expand Up @@ -86,7 +89,7 @@ default String eth_chainId() {

String eth_getTransactionCount(String address, String blockId) throws Exception ;

String eth_getBlockTransactionCountByHash(String blockHash)throws Exception;
String eth_getBlockTransactionCountByHash(BlockHashParam blockHash)throws Exception;

String eth_getBlockTransactionCountByNumber(String bnOrId)throws Exception;

Expand All @@ -108,19 +111,19 @@ default String eth_sendTransaction(CallArguments args) {
return getEthModule().sendTransaction(args);
}

BlockResultDTO eth_getBlockByHash(String blockHash, Boolean fullTransactionObjects) throws Exception;
BlockResultDTO eth_getBlockByHash(BlockHashParam blockHash, Boolean fullTransactionObjects) throws Exception;

BlockResultDTO eth_getBlockByNumber(String bnOrId, Boolean fullTransactionObjects) throws Exception;

TransactionResultDTO eth_getTransactionByHash(String transactionHash) throws Exception;
TransactionResultDTO eth_getTransactionByHash(TxHashParam transactionHash) throws Exception;

TransactionResultDTO eth_getTransactionByBlockHashAndIndex(String blockHash, String index) throws Exception;
TransactionResultDTO eth_getTransactionByBlockHashAndIndex(BlockHashParam blockHash, HexIndexParam index) throws Exception;

TransactionResultDTO eth_getTransactionByBlockNumberAndIndex(String bnOrId, String index) throws Exception;

TransactionReceiptDTO eth_getTransactionReceipt(String transactionHash) throws Exception;

BlockResultDTO eth_getUncleByBlockHashAndIndex(String blockHash, String uncleIdx) throws Exception;
BlockResultDTO eth_getUncleByBlockHashAndIndex(BlockHashParam blockHash, HexIndexParam uncleIdx) throws Exception;

BlockResultDTO eth_getUncleByBlockNumberAndIndex(String blockId, String uncleIdx) throws Exception;

Expand Down
14 changes: 12 additions & 2 deletions rskj-core/src/main/java/co/rsk/util/HexUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@
import org.bouncycastle.util.encoders.Hex;
import org.ethereum.util.ByteUtil;

import static org.ethereum.rpc.exception.RskJsonRpcRequestException.invalidParamError;

import java.math.BigInteger;
import java.nio.charset.StandardCharsets;
import java.util.regex.Pattern;

import static org.ethereum.rpc.exception.RskJsonRpcRequestException.invalidParamError;

/**
* Hex utils
*/
Expand Down Expand Up @@ -362,4 +362,14 @@
return Integer.parseInt(preResult, 16);
}

public static int jsonHexToIntOptionalPrefix(final String param) {
if (!hasHexPrefix(param) && !HexUtils.isHex(param)) {
throw invalidParamError(INCORRECT_HEX_SYNTAX);
}

String preResult = removeHexPrefix(param);

return Integer.parseInt(preResult, 16);

Check notice

Code scanning / CodeQL

Missing catch of NumberFormatException Note

Potential uncaught 'java.lang.NumberFormatException'.
}

}
83 changes: 47 additions & 36 deletions rskj-core/src/main/java/org/ethereum/rpc/Web3Impl.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@
import org.ethereum.rpc.dto.CompilationResultDTO;
import org.ethereum.rpc.dto.TransactionReceiptDTO;
import org.ethereum.rpc.dto.TransactionResultDTO;
import org.ethereum.rpc.parameters.BlockHashParam;
import org.ethereum.rpc.parameters.HexIndexParam;
import org.ethereum.rpc.parameters.TxHashParam;
import org.ethereum.util.BuildInfo;
import org.ethereum.vm.DataWord;
import org.slf4j.Logger;
Expand All @@ -75,9 +78,7 @@

import static co.rsk.util.HexUtils.*;
import static java.lang.Math.max;
import static org.ethereum.rpc.exception.RskJsonRpcRequestException.blockNotFound;
import static org.ethereum.rpc.exception.RskJsonRpcRequestException.invalidParamError;
import static org.ethereum.rpc.exception.RskJsonRpcRequestException.unimplemented;
import static org.ethereum.rpc.exception.RskJsonRpcRequestException.*;

public class Web3Impl implements Web3 {
private static final Logger logger = LoggerFactory.getLogger("web3");
Expand Down Expand Up @@ -235,8 +236,7 @@ public String net_version() {
try {
byte netVersion = config.getNetworkConstants().getChainId();
return s = Byte.toString(netVersion);
}
finally {
} finally {
if (logger.isDebugEnabled()) {
logger.debug("net_version(): {}", s);
}
Expand Down Expand Up @@ -411,10 +411,10 @@ public String eth_getCode(String address, Map<String, String> inputs) {
@Override
public String eth_getBalance(String address, String block) {
/* HEX String - an integer block number
* String "earliest" for the earliest/genesis block
* String "latest" - for the latest mined block
* String "pending" - for the pending state/transactions
*/
* String "earliest" for the earliest/genesis block
* String "latest" - for the latest mined block
* String "pending" - for the pending state/transactions
*/

AccountInformationProvider accountInformationProvider = web3InformationRetriever.getInformationProvider(block);

Expand Down Expand Up @@ -445,7 +445,7 @@ public String eth_getBalance(String address) {

@Override
public String eth_getStorageAt(String address, String storageIdx, Map<String, String> blockRef) {
return invokeByBlockRef(blockRef, blockNumber -> this.eth_getStorageAt(address,storageIdx, blockNumber));
return invokeByBlockRef(blockRef, blockNumber -> this.eth_getStorageAt(address, storageIdx, blockNumber));
}

@Override
Expand Down Expand Up @@ -485,16 +485,17 @@ public String eth_getTransactionCount(String address, Map<String, String> inputs
* It processes inputs maps ex: { "blockNumber": "0x0" },
* { "blockHash": "0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3", "requireCanonical": false }
* and invoke a function after processing.
* @param inputs map
*
* @param inputs map
* @param toInvokeByBlockNumber a function that returns a string based on the block number
* @return function invocation result
*/
protected String invokeByBlockRef(Map<String, String> inputs, UnaryOperator<String> toInvokeByBlockNumber) {
final boolean requireCanonical = Boolean.parseBoolean(inputs.get("requireCanonical"));
return applyIfPresent(inputs, "blockHash", blockHash -> this.toInvokeByBlockHash(blockHash, requireCanonical, toInvokeByBlockNumber))
.orElseGet(() -> applyIfPresent(inputs, "blockNumber", toInvokeByBlockNumber)
.orElseThrow(() -> invalidParamError("Invalid block input"))
);
.orElseThrow(() -> invalidParamError("Invalid block input"))
);
}

private String toInvokeByBlockHash(String blockHash, boolean requireCanonical, Function<String, String> toInvokeByBlockNumber) {
Expand Down Expand Up @@ -537,10 +538,10 @@ public Block getBlockByJSonHash(String blockHash) {
}

@Override
public String eth_getBlockTransactionCountByHash(String blockHash) {
public String eth_getBlockTransactionCountByHash(BlockHashParam blockHash) {
String s = null;
try {
Block b = getBlockByJSonHash(blockHash);
Block b = blockchain.getBlockByHash(blockHash.getHash().getBytes());

if (b == null) {
return null;
Expand Down Expand Up @@ -644,10 +645,14 @@ public BlockInformationResult[] eth_getBlocksByNumber(String number) {
}

@Override
public BlockResultDTO eth_getBlockByHash(String blockHash, Boolean fullTransactionObjects) {
public BlockResultDTO eth_getBlockByHash(BlockHashParam blockHash, Boolean fullTransactionObjects) {
if (blockHash == null) {
throw invalidParamError("blockHash is null");
}

BlockResultDTO s = null;
try {
Block b = getBlockByJSonHash(blockHash);
Block b = this.blockchain.getBlockByHash(blockHash.getHash().getBytes());
s = (b == null ? null : getBlockResult(b, fullTransactionObjects));
return s;
} finally {
Expand Down Expand Up @@ -675,16 +680,16 @@ public BlockResultDTO eth_getBlockByNumber(String bnOrId, Boolean fullTransactio
}

@Override
public TransactionResultDTO eth_getTransactionByHash(String transactionHash) {
public TransactionResultDTO eth_getTransactionByHash(TxHashParam transactionHash) {
TransactionResultDTO s = null;
try {
Keccak256 txHash = new Keccak256(stringHexToByteArray(transactionHash));
Keccak256 txHash = transactionHash.getHash();
Block block = null;

TransactionInfo txInfo = this.receiptStore.getInMainChain(txHash.getBytes(), blockStore).orElse(null);

if (txInfo == null) {
List<Transaction> txs = web3InformationRetriever.getTransactions("pending");
List<Transaction> txs = web3InformationRetriever.getTransactions("pending");

for (Transaction tx : txs) {
if (tx.getHash().equals(txHash)) {
Expand Down Expand Up @@ -713,16 +718,24 @@ public TransactionResultDTO eth_getTransactionByHash(String transactionHash) {
}

@Override
public TransactionResultDTO eth_getTransactionByBlockHashAndIndex(String blockHash, String index) {
public TransactionResultDTO eth_getTransactionByBlockHashAndIndex(BlockHashParam blockHash, HexIndexParam index) {
if (blockHash == null) {
throw invalidParamError("blockHash is null");
}

if (index == null) {
throw invalidParamError("index is null");
}

TransactionResultDTO s = null;
try {
Block b = getBlockByJSonHash(blockHash);
Block b = blockchain.getBlockByHash(blockHash.getHash().getBytes());

if (b == null) {
return null;
}

int idx = jsonHexToInt(index);
int idx = index.getIndex();

if (idx >= b.getTransactionsList().size()) {
return null;
Expand Down Expand Up @@ -782,17 +795,17 @@ public TransactionReceiptDTO eth_getTransactionReceipt(String transactionHash) {
}

@Override
public BlockResultDTO eth_getUncleByBlockHashAndIndex(String blockHash, String uncleIdx) {
public BlockResultDTO eth_getUncleByBlockHashAndIndex(BlockHashParam blockHash, HexIndexParam uncleIdx) {
BlockResultDTO s = null;

try {
Block block = blockchain.getBlockByHash(stringHexToByteArray(blockHash));
Block block = blockchain.getBlockByHash(blockHash.getHash().getBytes());

if (block == null) {
return null;
}

s = getUncleResultDTO(uncleIdx, block);
s = getUncleResultDTO(uncleIdx.getIndex(), block);

return s;
} finally {
Expand All @@ -802,14 +815,13 @@ public BlockResultDTO eth_getUncleByBlockHashAndIndex(String blockHash, String u
}
}

private BlockResultDTO getUncleResultDTO(String uncleIdx, Block block) {
int idx = jsonHexToInt(uncleIdx);
private BlockResultDTO getUncleResultDTO(Integer uncleIdx, Block block) {

if (idx >= block.getUncleList().size()) {
if (uncleIdx >= block.getUncleList().size()) {
return null;
}

BlockHeader uncleHeader = block.getUncleList().get(idx);
BlockHeader uncleHeader = block.getUncleList().get(uncleIdx);
Block uncle = blockchain.getBlockByHash(uncleHeader.getHash().getBytes());

if (uncle == null) {
Expand All @@ -829,8 +841,8 @@ public BlockResultDTO eth_getUncleByBlockNumberAndIndex(String blockId, String u
if (!block.isPresent()) {
return null;
}

s = getUncleResultDTO(uncleIdx, block.get());
int idx = jsonHexToInt(uncleIdx);
s = getUncleResultDTO(idx, block.get());

return s;
} finally {
Expand Down Expand Up @@ -871,7 +883,7 @@ public Map<String, CompilationResultDTO> eth_compileSolidity(String contract) {
public String eth_newFilter(FilterRequest fr) throws Exception {
String str = null;
try {
Filter filter = LogFilter.fromFilterRequest(fr, blockchain, blocksBloomStore, config.getRpcEthGetLogsMaxBlockToQuery(),config.getRpcEthGetLogsMaxLogsToReturn());
Filter filter = LogFilter.fromFilterRequest(fr, blockchain, blocksBloomStore, config.getRpcEthGetLogsMaxBlockToQuery(), config.getRpcEthGetLogsMaxLogsToReturn());
int id = filterManager.registerFilter(filter);

str = toQuantityJsonHex(id);
Expand Down Expand Up @@ -1103,7 +1115,7 @@ public RskModule getRskModule() {
/**
* Adds an address or block to the list of banned addresses
* It supports IPV4 and IPV6 addresses with an optional number of bits to ignore
*
* <p>
* "192.168.51.1" is a valid address
* "192.168.51.1/16" is a valid block
*
Expand All @@ -1125,7 +1137,7 @@ public void sco_banAddress(String address) {
/**
* Removes an address or block to the list of banned addresses
* It supports IPV4 and IPV6 addresses with an optional number of bits to ignore
*
* <p>
* "192.168.51.1" is a valid address
* "192.168.51.1/16" is a valid block
*
Expand Down Expand Up @@ -1182,7 +1194,6 @@ public PeerScoringReputationSummary sco_reputationSummary() {
* Clears scoring for the received id
*
* @param id peer identifier: firstly tried as an InetAddress, used as a NodeId otherwise
*
* @return the list of scoring information, per node id and address
*/
@SuppressWarnings("squid:S1166")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* This file is part of RskJ
* Copyright (C) 2023 RSK Labs Ltd.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.ethereum.rpc.parameters;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.deser.std.StdDeserializer;

import java.io.IOException;

@JsonDeserialize(using = BlockHashParam.Deserializer.class)
public class BlockHashParam extends HashParam32 {

private static final String HASH_TYPE = "block hash";

public BlockHashParam(String hash) {
super(HASH_TYPE, hash);
}

public static class Deserializer extends StdDeserializer<BlockHashParam> {

private static final long serialVersionUID = 8071595037666226210L;

public Deserializer() {
this(null);
}

public Deserializer(Class<?> vc) {
super(vc);
}

@Override
public BlockHashParam deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException {
String hash = jp.getText();
return new BlockHashParam(hash);
}
}
}
Loading