code-423n4 / 2023-03-canto-identity-findings

1 stars 1 forks source link

Potential front-running attacks in `buy` function #244

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-canto-identity/blob/main/canto-namespace-protocol/src/Tray.sol#L150

Vulnerability details

Impact

The buy function generates new trays and stores them in the tiles mapping based on the value of lastHash. Since the value of lastHash is publicly accessible and can be predicted, an attacker could potentially front-run other users to mint specific trays.

An attacker could monitor the value of lastHash and calculate the trays that will be generated. If they find a valuable tray, they can front-run other users by submitting a transaction with a higher gas price to ensure they mint that specific tray before others. This would give them an unfair advantage in obtaining valuable trays, undermining the fairness of the system.

Tools Used

Manual review

Recommended Mitigation Steps

To mitigate the potential front-running attacks in the buy function, a commit-reveal scheme can be implemented. This scheme adds a layer of unpredictability to the tray generation process, making it more difficult for an attacker to front-run other users. Here's a high-level overview of how to implement the commit-reveal scheme:

  1. Add a new mapping to store commitments:

    mapping(address => bytes32) public commitments;
  2. Add a new function for users to commit their intent to buy trays:

    function commit(bytes32 _commitment) external {
    commitments[msg.sender] = _commitment;
    }

    Here, _commitment is the hash of a secret value (chosen by the user) concatenated with a random nonce. This commitment is stored in the commitments mapping.

  3. Modify the buy function to accept a secret value and a nonce as parameters:

    function buy(uint256 _amount, uint256 _secret, uint256 _nonce) external {
    ...
    }
  4. In the buy function, verify that the commitment is valid before generating trays:

    bytes32 expectedCommitment = keccak256(abi.encodePacked(_secret, _nonce));
    require(commitments[msg.sender] == expectedCommitment, "Invalid commitment");
  5. Use the secret value as an additional input to the tray generation process:

    lastHash = keccak256(abi.encode(lastHash, _secret));
    trayTiledata[j] = _drawing(uint256(lastHash));

    .

  6. Clear the user's commitment after successfully generating trays:

    commitments[msg.sender] = bytes32(0);

This commit-reveal scheme requires users to first commit their intent to buy trays, and then reveal their secret value and nonce when actually buying the trays. The secret value is used as an additional input to the tray generation process, making it more difficult for attackers to predict and front-run specific trays

c4-judge commented 1 year ago

0xleastwood marked the issue as duplicate of #121

c4-judge commented 1 year ago

0xleastwood marked the issue as satisfactory

c4-judge commented 1 year ago

0xleastwood marked the issue as selected for report

c4-judge commented 1 year ago

0xleastwood marked the issue as duplicate of #121

c4-judge commented 1 year ago

0xleastwood marked the issue as not selected for report

c4-judge commented 1 year ago

0xleastwood marked the issue as duplicate of #130

0xleastwood commented 1 year ago

This solution doesn't make a lot of sense because users could still predict the hash by altering the secret and nonce. It would therefore be possible to control the tray data.

c4-judge commented 1 year ago

0xleastwood marked the issue as partial-25