code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-10-thegraph/blob/7ea88cc41f17f2d49961aafec7ebe72daeaad3f9/contracts/gateway/L1GraphTokenGateway.sol#L214

Vulnerability details

Impact

Whitelisted entity cannot use callhook if they use router

Proof of Concept

In the L1GraphTokenGateway, the callhookWhitelist was checked for the msg.sender. If a whitelisted entity is using router, the router will be the msg.sender and the whitelisted entity is the from. Thus, they cannot use the callhook.

// L1GraphTokenGateway
// outboundTransfer

212                 (from, maxSubmissionCost, extraData) = parseOutboundData(_data);
213                 require(
214                     extraData.length == 0 || callhookWhitelist[msg.sender] == true,
215                     "CALL_HOOK_DATA_NOT_ALLOWED"
216                 );

Tools Used

Manual review

Recommended Mitigation Steps

consider check the callhookWhitelist for the from address.

trust1995 commented 2 years ago

Seems like the contract is not working as intended.

0xean commented 2 years ago

I don't believe that the design intends for the whitelisted addresses to use the router. Will leave open for sponsor review and confirmation, but most likely this is invalid.

pcarranzav commented 2 years ago

The Gateway Router will generally only be used by end users bridging tokens through the Arbitrum bridge UI. Whitelisted contracts will only perform direct calls to the L1GraphTokenGateway, so indeed there's no need to allow messages from the router to trigger callhooks.