search

code-423n4 / 2022-10-thegraph-findings

0 stars 0 forks source link

finalizeInboundTransfer() may have reentrant attacking risk #192

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/gateway/L1GraphTokenGateway.sol#L269

Vulnerability details

Impact

finalizeInboundTransfer() may be attacked in reentrant ways.

Proof of Concept

finalizeInboundTransfer() is supposed to transfer token from escrow contract to _to address. However, this function is not protected with NonReentrant modifier. So if the _to address or other parties implemented malicious logic to perform reentrance attack, it may drain out the token in the escrow address.

Tools Used

Recommended Mitigation Steps

Add NonReentrant modifier to the function signature, as shown below.

function finalizeInboundTransfer( ) external payable override notPaused onlyL2Counterpart nonReentrant{

trust1995 commented 1 year ago

The only call in this function is to graphtoken's transferFrom(), which does not support any hooks, so no impact is in sight.

0xean commented 1 year ago

closing as invalid.