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

withdrawTokens()::Bridge.sol shouldn't be payable

0xziin

Summary

withdrawTokens()::Bridge.sol shouldn't be payable. It allows the end user to send native tokens when executing this function but no native token is needed to withdraw NFTs.

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/Bridge.sol#L153-L215

Vulnerability Details

The function depositTokens() is payable because it uses msg.value :

IStarknetMessaging(_starknetCoreAddress).sendMessageToL2{value: msg.value}(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload
);

But withdrawTokens() doesn't use msg.value in its code, and doesn't have to. No need to send native tokens to the contract when using this function to withdraw NFTs.

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/Bridge.sol#L157

If an end user send some native tokens by mistake using this withdrawTokens() function, those will be lost.

I looked into LightChaserV3_CodeHawks_Ark.mdto see if this bug was already found, there are 2 similar findings but not for this particular function : withdrawTokens()::Bridge.sol.

These are those 2 findings from LightChaserV3 which are not relevant :

https://github.com/Cyfrin/2024-07-ark-project/tree/main/apps/blockchain/ethereum/src/Messaging.sol#L28-L28
28: contract StarklaneMessaging is Ownable

https://github.com/Cyfrin/2024-07-ark-project/tree/main/apps/blockchain/ethereum/src/UUPSProxied.sol#L14-L14
14: contract UUPSOwnableProxied is Ownable, UUPSUpgradeable

/**
@notice Ensures unsupported function is directly reverted.
*/
fallback()
external
payable
{
revert NotSupportedError();
}
​
/**
@notice Ensures no ether is received without a function call.
*/
receive()
external
payable
{
revert NotPayableError();
}

LightChaserV3 highlighted those 2 contracts but like we can see, it is not relevant. But it doesn't found withdrawTokens()::Bridge.sol, which is in fact the only one causing problem.

Impact

Possible loss of funds for the end user.

Tools Used

Github, VisualCode.

Recommendations

payable should be removed from :

function withdrawTokens( uint256[] calldata request ) external payable returns (address) { ... }
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.

Support

FAQs

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