fix(connector-quorum/ethereum): strengthen contract parameter validation

[petermetz]

Description

Static source code assessment has picked up a potential code injection vulnerability
in the code of the Quorum and Ethereum connectors where we pass in an array of
user-provided arguments to the EVM contract methods.

Web3JS already has validation on the number of arguments, but we could fortify
things on our end regardless by doing

1. Additional type checking
2. Ensuring that the number of arguments are the correct one (if we can obtain
   this information somehow from the ABI)

The report from which the above information was summarized

Risk Rating: High

Category: Injection

Description

The application's ‘invokeRawWeb3EthContract method’ receives and dynamically executes user-controlled code using invocationParams. This could enable an attacker to inject and run arbitrary code. The attacker can inject the executed code via user input, body, which is retrieved by the application in the handleRequest method.

Impact

An attacker could run arbitrary code on the application server host. Depending on the application’s OS permissions,these could include:

• Database access, such as reading or modifying sensitive data.
• File actions (read / create / modify / delete).
• Changing the website
• Open a network connection to the attacker’s server.
• Decrypt secret data using the application's encryption keys.
• Start and stop system services.
• Complete server takeover.

Remediation Recommendation

• The application should not compile, execute, or evaluate any untrusted code from any external source, including user input, uploaded files, or a database.
• If it is absolutely necessary to include external data in dynamic execution, it is permissible to pass the data as parameters to the code, but do not execute user data directly.
• If it is necessary to pass untrusted data to dynamic execution, enforce very strict data validation. For example, accept only integers between certain values.
• Validate all input, regardless of source. Validation should be based on a whitelist: accept only data fitting a specified structure, rather than reject bad patterns. Parameters should be limited to an allowed character set, and non-validated input should be dropped. In addition to characters, check for:
  • Data type
  • Size
  • Range
  • Format
  • Expected values
• If possible, always prefer to whitelist known and trusted input instead of comparing to a blacklist.
• Configure the application to run using a restricted user account that has no unnecessary privileges.
• If possible, isolate all dynamic execution to use a separate, dedicated user account that has privileges only for the specific operations and files used by dynamic execution, according to the Principle of Least Privilege.
• Prefer passing user data to a pre-implemented script, e.g. in another isolated application, over dynamically executing user-controlled code.

Affected Files

Path
Line No

• packages\cactus-plugin-ledger-connector-quorum\src\main\typescript\web-services\invoke-raw-web3eth-contract-v1-endpoint.ts.

  • Line number: 91

• packages\cactus-plugin-ledger-connector-quorum\src\main\typescript\plugin-ledger-connector-quorum.ts

  • Line number: 904

• packages\cactus-plugin-ledger-connector-ethereum\src\main\typescript\web-services\invoke-raw-web3eth-contract-v1-endpoint.ts.

  • Line number: 91

• packages\cactus-plugin-ledger-connector-ethereum\src\main\typescript\plugin-ledger-connector-ethereum.ts.

  • Line number: 800

Snapshot of source code at the time of scan

---

Source: APP PE Hyperledger Cacti v2.0.0 - Static Application Assessment Report.odt

cc: @takeutak @izuru0 @outSH

[outSH]

Good idea, we could use @ethersproject/abi to get function inputs from ABI and validate it's type - https://docs.ethers.org/v5/api/utils/abi/fragments/#FunctionFragment

[shivam-Purohit]

hey @petermetz should I give it a try?

[petermetz]

@shivam-Purohit Fine by me, yes! Thank you for your contribution in advance!