Skip to content

Commit

Permalink
fix(connector-quorum/ethereum): strengthen contract parameter validation
Browse files Browse the repository at this point in the history
Peter's updates:
1. Made improvements to the test case verifying that the parameters with
incorrect types are indeed being rejected with useful error messaging
2. Added a new library (which I also had to re-publish with CJS exports)

Fixes #2760

Signed-off-by: Shivam Purohit <[email protected]>
Signed-off-by: Peter Somogyvari <[email protected]>
  • Loading branch information
shivam-Purohit authored and petermetz committed Jan 25, 2024
1 parent 6efd8de commit f299b48
Show file tree
Hide file tree
Showing 10 changed files with 217 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ export function consensusHasTransactionFinality(
!isInConsensusAlgorithmFamiliesWithTxFinality &&
!isInConsensusAlgorithmFamiliesWithOutTxFinality;

if (unrecognizedConsensusAlgorithmFamily) {
throw new BadRequestError(
`Unrecognized consensus algorithm family: ${consensusAlgorithmFamily}`,
{
acceptedValuesCsv,
},
);
}
return isInConsensusAlgorithmFamiliesWithTxFinality;
if (unrecognizedConsensusAlgorithmFamily) {
throw new BadRequestError(
`Unrecognized consensus algorithm family: ${consensusAlgorithmFamily}`,
{
acceptedValuesCsv,
},
);
}
return isInConsensusAlgorithmFamiliesWithTxFinality;
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
"@types/uuid": "9.0.6",
"body-parser": "1.20.2",
"chalk": "4.1.2",
"ethers": "6.8.1",
"js-yaml": "4.1.0",
"socket.io": "4.5.4",
"uuid": "9.0.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { PayableMethodObject } from "web3-eth-contract";

import OAS from "../json/openapi.json";

import { Interface, FunctionFragment, isAddress } from "ethers";

import {
ConsensusAlgorithmFamily,
IPluginLedgerConnector,
Expand Down Expand Up @@ -1126,6 +1128,53 @@ export class PluginLedgerConnectorEthereum
`Invalid method name provided in request. ${args.contractMethod} does not exist on the Web3 contract object's "methods" property.`,
);
}
const abiInterface = new Interface(args.abi);
const methodFragment: FunctionFragment | null = abiInterface.getFunction(
args.contractMethod,
);
if (!methodFragment) {
throw new RuntimeError(
`Method ${args.contractMethod} not found in ABI interface.`,
);
}

// validation for the contractMethod
if (methodFragment.inputs.length !== contractMethodArgs.length) {
throw new Error(
`Incorrect number of arguments for ${args.contractMethod}`,
);
}
methodFragment.inputs.forEach((input, index) => {
const argValue = contractMethodArgs[index];
const isValidType = typeof argValue === input.type;

if (!isValidType) {
throw new Error(
`Invalid type for argument ${index + 1} in ${args.contractMethod}`,
);
}
});

//validation for the invocationParams
const invocationParams = args.invocationParams as Record<string, any>;
const allowedKeys = ["from", "gasLimit", "gasPrice", "value"];

if (invocationParams) {
Object.keys(invocationParams).forEach((key) => {
if (!allowedKeys.includes(key)) {
throw new Error(`Invalid key '${key}' in invocationParams`);
}
if (key === "from" && !isAddress(invocationParams[key])) {
throw new Error(`Invalid type for 'from' in invocationParams`);
}
if (key === "gasLimit" && typeof invocationParams[key] !== "number") {
throw new Error(`Invalid type for '${key}' in invocationParams`);
}
if (key === "gasPrice" && typeof invocationParams[key] !== "number") {
throw new Error(`Invalid type for '${key}'in invocationParams`);
}
});
}

const methodRef = contract.methods[args.contractMethod] as (
...args: unknown[]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ export class InvokeRawWeb3EthContractEndpoint implements IWebServiceEndpoint {
public async handleRequest(req: Request, res: Response): Promise<void> {
const reqTag = `${this.getVerbLowerCase()} - ${this.getPath()}`;
this.log.debug(reqTag);

try {
const methodResponse =
await this.options.connector.invokeRawWeb3EthContract(req.body);
Expand Down
3 changes: 3 additions & 0 deletions packages/cactus-plugin-ledger-connector-quorum/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"@hyperledger/cactus-core-api": "2.0.0-alpha.2",
"axios": "1.6.0",
"express": "4.18.2",
"http-errors-enhanced-cjs": "2.0.0",
"minimist": "1.2.8",
"prom-client": "13.2.0",
"run-time-error-cjs": "1.4.0",
Expand All @@ -80,11 +81,13 @@
"@hyperledger/cactus-test-tooling": "2.0.0-alpha.2",
"@types/body-parser": "1.19.4",
"@types/express": "4.17.19",
"@types/http-errors": "2.0.4",
"@types/minimist": "1.2.2",
"@types/sanitize-html": "2.6.2",
"@types/uuid": "9.0.6",
"body-parser": "1.20.2",
"chalk": "4.1.2",
"ethers": "6.8.1",
"socket.io": "4.5.4",
"uuid": "9.0.1",
"web3-core": "1.6.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@ import { Contract } from "web3-eth-contract";
import { ContractSendMethod } from "web3-eth-contract";
import { TransactionReceipt } from "web3-eth";

import { BadRequestError } from "http-errors-enhanced-cjs";

import OAS from "../json/openapi.json";

import { Interface, FunctionFragment, isAddress } from "ethers";

import {
ConsensusAlgorithmFamily,
IPluginLedgerConnector,
Expand Down Expand Up @@ -884,9 +888,8 @@ export class PluginLedgerConnectorQuorum
args.invocationType,
)
) {
throw new Error(
`Unknown invocationType (${args.invocationType}), must be specified in EthContractInvocationWeb3Method`,
);
const eMsg = `Unknown invocationType (${args.invocationType}), must be specified in EthContractInvocationWeb3Method`;
throw new BadRequestError(eMsg);
}

const contract = new this.web3.eth.Contract(
Expand All @@ -899,13 +902,66 @@ export class PluginLedgerConnectorQuorum
args.contractMethod,
);
if (!isSafeToCall) {
throw new RuntimeError(
`Invalid method name provided in request. ${args.contractMethod} does not exist on the Web3 contract object's "methods" property.`,
);
const msg = `Invalid method name provided in request. ${args.contractMethod} does not exist on the Web3 contract object's "methods" property.`;
throw new BadRequestError(msg);
}
const abiInterface = new Interface(args.abi);
const methodFragment: FunctionFragment | null = abiInterface.getFunction(
args.contractMethod,
);
if (!methodFragment) {
const msg = `Method ${args.contractMethod} not found in ABI interface.`;
throw new BadRequestError(msg);
}

// validation for the contractMethod
if (methodFragment.inputs.length !== contractMethodArgs.length) {
const msg = `Incorrect number of arguments for ${args.contractMethod}`;
throw new BadRequestError(msg);
}
methodFragment.inputs.forEach((input, index) => {
const argValue = contractMethodArgs[index];
const isValidType = typeof argValue === input.type;

if (!isValidType) {
const msg = `Invalid type for argument ${index + 1} in ${
args.contractMethod
}`;
throw new BadRequestError(msg);
}
});

//validation for the invocationParams
const invocationParams = args.invocationParams as Record<string, unknown>;
const allowedKeys = ["from", "gasLimit", "gasPrice", "value"];

if (invocationParams) {
Object.keys(invocationParams).forEach((key) => {
if (!allowedKeys.includes(key)) {
throw new BadRequestError(`Invalid key '${key}' in invocationParams`);
}
if (key === "from" && !isAddress(invocationParams[key])) {
throw new BadRequestError(
`Invalid type for 'from' in invocationParams`,
);
}
if (key === "gasLimit" && typeof invocationParams[key] !== "number") {
throw new BadRequestError(
`Invalid type for '${key}' in invocationParams`,
);
}
if (key === "gasPrice" && typeof invocationParams[key] !== "number") {
throw new BadRequestError(
`Invalid type for '${key}'in invocationParams`,
);
}
});
}

return contract.methods[args.contractMethod](...contractMethodArgs)[
args.invocationType
](args.invocationParams);
const txObjectFactory = contract.methods[args.contractMethod];
const txObject = txObjectFactory(...contractMethodArgs);

Check failure

Code scanning / CodeQL

Unvalidated dynamic method call High

Invocation of method with
user-controlled
name may dispatch to unexpected target and cause an exception.
const executor = txObject[args.invocationType];
const output = await executor(args.invocationParams);
return output;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ export class InvokeRawWeb3EthContractEndpoint implements IWebServiceEndpoint {
public async handleRequest(req: Request, res: Response): Promise<void> {
const reqTag = `${this.getVerbLowerCase()} - ${this.getPath()}`;
this.log.debug(reqTag);

try {
const methodResponse =
await this.options.connector.invokeRawWeb3EthContract(req.body);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,20 @@ const contractName = "HelloWorld";
const testCase = "Quorum Ledger Connector Plugin";

describe(testCase, () => {
const containerImageVersion = "2021-05-03-quorum-v21.4.1";
const ledgerOptions = { containerImageVersion };
const ledger = new QuorumTestLedger(ledgerOptions);

afterAll(async () => {
await ledger.stop();
await ledger.destroy();
});

const containerImageVersion = "2021-05-03-quorum-v21.4.1";
const ledgerOptions = { containerImageVersion };
const ledger = new QuorumTestLedger(ledgerOptions);

test(testCase, async () => {
beforeAll(async () => {
await ledger.start();
});

test("can invoke contract methods", async () => {
const rpcApiHttpHost = await ledger.getRpcApiHttpHost();
const quorumGenesisOptions: IQuorumGenesisOptions =
await ledger.getGenesisJsObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ describe("invokeRawWeb3EthContract Tests", () => {
},
gas: 1000000,
});
log.debug("Contract deployed OK");
expect(deployOut).toBeTruthy();
expect(deployOut.transactionReceipt).toBeTruthy();
expect(deployOut.transactionReceipt.contractAddress).toBeTruthy();
Expand Down Expand Up @@ -199,4 +200,22 @@ describe("invokeRawWeb3EthContract Tests", () => {

await expect(connector.invokeRawWeb3EthContract(callInvokeArgs)).toReject();
});

it("validates input parameters based on their solidity type declarations", async () => {
const req: InvokeRawWeb3EthContractV1Request = {
abi: contractAbi,
address: contractAddress,
invocationType: EthContractInvocationWeb3Method.Call,
contractMethod: "setName",
contractMethodArgs: [42],
invocationParams: {
gasLimit: 999999,
},
};

const eMsgExpected = `Invalid type for argument ${0 + 1} in ${"setName"}`;

const theInvocation = connector.invokeRawWeb3EthContract(req);
await expect(theInvocation).rejects.toThrowWithMessage(Error, eMsgExpected);
});
});
Loading

0 comments on commit f299b48

Please sign in to comment.