ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

`Bridge::depositTokens(...)` function is vulnerable to replay attacks

x18a6

Summary

Bridge::depositTokens(...) function has is vulnerable to replay attacks.

Vulnerability Details

function depositTokens(
uint256 salt,
address collectionL1,
snaddress ownerL2,
uint256[] calldata ids,
bool useAutoBurn
) external payable {
...
Request memory req;
​
req.header = Protocol.requestHeaderV1(ctype, useAutoBurn, false);
req.hash = Protocol.requestHash(salt, collectionL1, ownerL2, ids);
...
}

ethereum/src/Bridge.sol#L110

There are few problems with the implementation as it stands:

  • salt is user-inputed: This means that a depositor could generate the same hash for two different deposits by using the same salt.

  • msg.sender (the depositor) is not part of the hash: This means that two different depositors could generate the same request hash if they use the same salt, collectionL1, ownerL2, and ids.

  • useAutoBurn is not part of the hash: This means that an attacker could potentially burn tokens for a user when they don't want to.

Impact

Described above. Note that the same applies for Starknet deposits.

Tools Used

Manual review.

Recommendations

  • Include msg.sender and useBurnAuto in the hash. This can be done by modifying the requestHash:

+ req.hash = Protocol.requestHash(salt, collectionL1, ownerL2, ids, msg.sender, useBurnAuto);
- req.hash = Protocol.requestHash(salt, collectionL1, ownerL2, ids);

  • Ensure that each salt is unique for each deposit:

+ if (isUsed[salt]) {
+ revert();
+ }
+ isUsed[salt] = true;

Or consider not making salt not user-inputted but incrementing the salt for each deposit.

Updates

Lead Judging Commences

n0kto Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
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.

Appeal created

x18a6 Submitter
11 months ago
x18a6 Submitter
11 months ago
n0kto Lead Judge
11 months ago
n0kto Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
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.