ArkProject: NFT Bridge

Summary

DOS of L1 bridge as users cannot deposit because of wrong token transfer flow.

Vulnerability Details

When users want to bridge tokens from ethereum to starknet they use the bridge contract on L1 to deposit by calling depositTokens() then it deposits the tokens into the escrow contract (Escrow.sol) https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L129 , the issue here is that in Escrow._depositIntoEscrow() tokens are transferred from msg.sender to the escrow contract as seen here https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Escrow.sol#L38-L47 but the msg.sender that the tokens are supposed to be transferred from, which is the bridge calling the escrow contract and it does not hold the tokens, the tokens are still held in the users EOA or whatever, the bridge is just approved to spend it. This call will always revert causing bridging from ethereum to starknet inaccessible permanently.

Impact

L1 bridge is useless and cannot deposit tokens to be bridged to starknet.

Tools Used

Manual Review

Recommendations

Change the msg.sender to req.ownerL1 in Escrow._depositIntoEscrow() .