Concur Finance contest - GeekyLumberjack's results

Lines of code

https://github.com/code-423n4/2022-02-concur/blob/main/contracts/Shelter.sol#L52-L59 https://github.com/code-423n4/2022-02-concur/commit/229b6188a2c5867f2d0cb4579d0a0d49516da252

Vulnerability details

Impact

withdraw() in Shelter.sol can be called multiple times by the same party. Eventually based on the users shareOf this would lead to complete drain of funds in Shelter.sol.

Proof of Concept

This requires that both msg.sender and _to addresses have funds that are now in the shelter. This would only be done in the case of emergency as said in the code4rena discord. An example of using the shelter was provided here.

1. Alice has 100 tokens in the Shelter and the total balance in the shelter is 1000
2. Alice calls withdraw and receives 100 tokens.
3. Alice calls withdraw 9 more times and receives all 1000 tokens that were in the shelter

This happens because claimed[_token][_to] = true; is set but not checked

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add a require statement to check if the address has claimed their tokens already.

require(claimed[_token][_to] == false, "already claimed")