Skip to content

Commit

Permalink
fix(connector-besu/quorum/xdai): unvalidated dynamic method call
Browse files Browse the repository at this point in the history
Added checks to make sure that the Web3 Contract instances
"methods" object has a property of their own called
the same way the method is called by the request
object. This way if someone tries to execute malicious
code by providing method names that are designed to
execute something other than the smart contract methods
we throw back an error to them instead of complying.

This is needed to fix the following CodeQL security advisories:
https://github.com/hyperledger/cactus/security/code-scanning/23
https://github.com/hyperledger/cactus/security/code-scanning/24
https://github.com/hyperledger/cactus/security/code-scanning/25
https://github.com/hyperledger/cactus/security/code-scanning/26

Todo for later: create a web3-common package that can
be used to house re-usable pieces of code such as the
function that validates if a contract really has a certain
method or not. Right now this method is copy pasted
to all 3 web3 flavored connectors which is not very nice.

Fixes #1911

Signed-off-by: Peter Somogyvari <[email protected]>
  • Loading branch information
petermetz committed Mar 14, 2022
1 parent 190cf12 commit bdc1aba
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,39 @@ export class PluginLedgerConnectorBesu

return consensusHasTransactionFinality(currentConsensusAlgorithmFamily);
}

/**
* Verifies that it is safe to call a specific method of a Web3 Contract.
*
* @param contract The Web3 Contract instance to check whether it has a method with a specific name or not.
* @param name The name of the method that will be checked if it's usable on `contract` or not.
* @returns Boolean `true` when it IS safe to call the method named `name` on the contract.
* @throws If the contract instance is falsy or it's methods object is falsy. Also throws if the method name is a blank string.
*/
public async isSafeToCallContractMethod(
contract: Contract,
name: string,
): Promise<boolean> {
Checks.truthy(
contract,
`${this.className}#isSafeToCallContractMethod():contract`,
);

Checks.truthy(
contract.methods,
`${this.className}#isSafeToCallContractMethod():contract.methods`,
);

Checks.nonBlankString(
name,
`${this.className}#isSafeToCallContractMethod():name`,
);

const { methods } = contract;

return Object.prototype.hasOwnProperty.call(methods, name);
}

public async invokeContract(
req: InvokeContractV1Request,
): Promise<InvokeContractV1Response> {
Expand Down Expand Up @@ -392,6 +425,16 @@ export class PluginLedgerConnectorBesu
contractInstance = new this.web3.eth.Contract(abi, contractAddress);
}

const isSafeToCall = await this.isSafeToCallContractMethod(
contractInstance,
req.methodName,
);
if (!isSafeToCall) {
throw new RuntimeError(
`Invalid method name provided in request. ${req.methodName} does not exist on the Web3 contract object's "methods" property.`,
);
}

const methodRef = contractInstance.methods[req.methodName];
Checks.truthy(methodRef, `${fnTag} YourContract.${req.methodName}`);
const method: ContractSendMethod = methodRef(...req.params);
Expand Down Expand Up @@ -945,6 +988,17 @@ export class PluginLedgerConnectorBesu
}
const { contractAddress } = request.invokeCall;
const contractInstance = new this.web3.eth.Contract(abi, contractAddress);

const isSafeToCall = await this.isSafeToCallContractMethod(
contractInstance,
request.invokeCall.methodName,
);
if (!isSafeToCall) {
throw new RuntimeError(
`Invalid method name provided in request. ${request.invokeCall.methodName} does not exist on the Web3 contract object's "methods" property.`,
);
}

const methodRef = contractInstance.methods[request.invokeCall.methodName];
Checks.truthy(
methodRef,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import {
GetPrometheusExporterMetricsEndpointV1,
IGetPrometheusExporterMetricsEndpointV1Options,
} from "./web-services/get-prometheus-exporter-metrics-endpoint-v1";
import { RuntimeError } from "run-time-error";

export interface IPluginLedgerConnectorQuorumOptions
extends ICactusPluginOptions {
Expand Down Expand Up @@ -222,6 +223,38 @@ export class PluginLedgerConnectorQuorum
return consensusHasTransactionFinality(currentConsensusAlgorithmFamily);
}

/**
* Verifies that it is safe to call a specific method of a Web3 Contract.
*
* @param contract The Web3 Contract instance to check whether it has a method with a specific name or not.
* @param name The name of the method that will be checked if it's usable on `contract` or not.
* @returns Boolean `true` when it IS safe to call the method named `name` on the contract.
* @throws If the contract instance is falsy or it's methods object is falsy. Also throws if the method name is a blank string.
*/
public async isSafeToCallContractMethod(
contract: InstanceType<typeof Contract>,
name: string,
): Promise<boolean> {
Checks.truthy(
contract,
`${this.className}#isSafeToCallContractMethod():contract`,
);

Checks.truthy(
contract.methods,
`${this.className}#isSafeToCallContractMethod():contract.methods`,
);

Checks.nonBlankString(
name,
`${this.className}#isSafeToCallContractMethod():name`,
);

const { methods } = contract;

return Object.prototype.hasOwnProperty.call(methods, name);
}

public async getContractInfoKeychain(
req: InvokeContractV1Request,
): Promise<any> {
Expand Down Expand Up @@ -306,6 +339,16 @@ export class PluginLedgerConnectorQuorum
contractAddress,
);

const isSafeToCall = await this.isSafeToCallContractMethod(
contractInstance,
req.methodName,
);
if (!isSafeToCall) {
throw new RuntimeError(
`Invalid method name provided in request. ${req.methodName} does not exist on the Web3 contract object's "methods" property.`,
);
}

const methodRef = contractInstance.methods[req.methodName];
Checks.truthy(methodRef, `${fnTag} YourContract.${req.methodName}`);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import {
} from "./web-services/get-prometheus-exporter-metrics-endpoint-v1";
import { DeployContractSolidityBytecodeJsonObjectEndpoint } from "./web-services/deploy-contract-solidity-bytecode-json-object-endpoint";
import { InvokeContractJsonObjectEndpoint } from "./web-services/invoke-contract-json-object-endpoint";
import { RuntimeError } from "run-time-error";

export const E_KEYCHAIN_NOT_FOUND = "cactus.connector.xdai.keychain_not_found";

Expand Down Expand Up @@ -227,6 +228,38 @@ export class PluginLedgerConnectorXdai
return ConsensusAlgorithmFamily.Authority;
}

/**
* Verifies that it is safe to call a specific method of a Web3 Contract.
*
* @param contract The Web3 Contract instance to check whether it has a method with a specific name or not.
* @param name The name of the method that will be checked if it's usable on `contract` or not.
* @returns Boolean `true` when it IS safe to call the method named `name` on the contract.
* @throws If the contract instance is falsy or it's methods object is falsy. Also throws if the method name is a blank string.
*/
public async isSafeToCallContractMethod(
contract: InstanceType<typeof Contract>,
name: string,
): Promise<boolean> {
Checks.truthy(
contract,
`${this.className}#isSafeToCallContractMethod():contract`,
);

Checks.truthy(
contract.methods,
`${this.className}#isSafeToCallContractMethod():contract.methods`,
);

Checks.nonBlankString(
name,
`${this.className}#isSafeToCallContractMethod():name`,
);

const { methods } = contract;

return Object.prototype.hasOwnProperty.call(methods, name);
}

async runInvoke(req: InvokeRequestBaseV1): Promise<InvokeContractV1Response> {
const fnTag = `${this.className}#runInvoke()`;

Expand All @@ -248,6 +281,16 @@ export class PluginLedgerConnectorXdai
contractAddress,
);

const isSafeToCall = await this.isSafeToCallContractMethod(
contractInstance,
req.methodName,
);
if (!isSafeToCall) {
throw new RuntimeError(
`Invalid method name provided in request. ${req.methodName} does not exist on the Web3 contract object's "methods" property.`,
);
}

const methodRef = contractInstance.methods[req.methodName];
Checks.truthy(methodRef, `${fnTag} YourContract.${req.methodName}`);

Expand Down

0 comments on commit bdc1aba

Please sign in to comment.