search

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

0 stars 0 forks source link

`L1GraphTokenGateway`, `L2GraphTokenGateway` does not enforce to set all the necessary variables before unpause #202

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-10-thegraph/blob/7ea88cc41f17f2d49961aafec7ebe72daeaad3f9/contracts/gateway/L1GraphTokenGateway.sol#L99-L102 https://github.com/code-423n4/2022-10-thegraph/blob/7ea88cc41f17f2d49961aafec7ebe72daeaad3f9/contracts/l2/gateway/L2GraphTokenGateway.sol#L87-L92

Vulnerability details

Impact

If the gateway would be unpaused before all the necessary variables are set, GRT could be locked.

Proof of Concept

Although the L1GraphTokenGateway and L2GraphTokenGateway are paused in the initialize function, they can unpaused before all the variables are set by mistake, as there is no check to ensure the variables are set before unpause.

If, for example in the L1GraphTokenGateway, l2Counterpart would have not been set, the message could be sent to zero, and the user cannot get GRT from the L2 side.

Tools Used

Manual review

Recommended Mitigation Steps

Add a function to check whether all the variables are set, and then unpause the gateway

0xean commented 1 year ago

Downgrading to QA, dupe of #198 - which is being used as wardens QA report

pcarranzav commented 1 year ago

This was actually also noted to us by OpenZeppelin (in a new audit, not the already referenced one), and I think it's a good one.