NFTBridge
60,000 USDC
View results
Submission Details
Severity: low
Invalid

The same hash can be used for multiple requests

Summary

When bridging NFT from L1 to starknet, the request hash is calculated by using abi.encodePacked method. A random salt is used to generate a unique hash everytime. However this can be bypassed in case:

  1. User bridged token id 1 from L1 to starknet using salt 0x1 and bridged to ownerL2 A.

  2. User bridged back the token from starknet to L1.

  3. User then proceed to bridge again the same token id 1 from L1 to starknet using the same salt 0x1 and bridged to ownerL2 A.

As a result request in step 1 and setp 3 will have the same hash because they are using the same tokenId, the same salt, are from the same sender and are being sent to the same L2 address

Vulnerability Details

Request hash is calculated inside Protocol.sol as follows:

function requestHash(
uint256 salt,
address collection,
snaddress toL2Address,
uint256[] memory tokenIds
)
internal
pure
returns (uint256)
{
bytes32 hash = keccak256(
abi.encodePacked(
salt,
// Cairo uses felts, which are converted into u256 to compute keccak.
// As we use abi.encodePacked, we want the address to also be 32 bytes long.
uint256(uint160(collection)),
snaddress.unwrap(toL2Address),
tokenIds
)
);
return uint256(hash);
}

And all the arguments are user supplied input. They are supplied inside Bridge.sol

function depositTokens(
uint256 salt,
address collectionL1,
snaddress ownerL2,
uint256[] calldata ids,
bool useAutoBurn
)
external
payable
{

This allows any user to supply the same input given he owns the tokens Ids. He can then proceed to create multiple request with the same hash as outlined in this report summary.

  1. User bridged token id 1 from L1 to starknet using salt 0x1 and bridged to ownerL2 A.

  2. User bridged back the token from starknet to L1.

  3. User then proceed to bridge again the same token id 1 from L1 to starknet using the same salt 0x1 and bridged to ownerL2 A.

Impact

User can process multiple request with the samee hash

Tools Used

Manual review

Recommendations

Instead of using a user supplied salt, use a mapping to create unique salt each time user calls it. It should be a uint that increment for the same user each time he create a request.

Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational / Gas

Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.

Appeal created

helium Submitter
about 1 year ago
n0kto Lead Judge
12 months ago
n0kto Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid-replay-attack-hash-not-stored-nonce-not-used

There is no impact here: Transaction cannot be replayed because the blockchain use the nonce in the signature. Hash is computed on-chain. Using or trying to have the same hash mean you need to buy the token, and they will be sent to their origin owner. Why an attacker would buy tokens to give them back ? No real impact.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.