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

0 stars 0 forks source link

Governor can approve many spenders to spend unlimited amount in ```BridgeEscrow``` contract #187

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/gateway/BridgeEscrow.sol#L24-L30

Vulnerability details

Impact

The governor can approve a set of spenders with unlimited amount of GRT.

09: /**
10:  * @title Bridge Escrow
11:  * @dev This contracts acts as a gateway for an L2 bridge (or several). It simply holds GRT and has
12:  * a set of spenders that can transfer the tokens; the L1 side of each L2 bridge has to be
13:  * approved as a spender.
14:  */
28:     function approveAll(address _spender) external onlyGovernor {
29:         graphToken().approve(_spender, type(uint256).max);
30:     }

This means a compromised spender address can steal the funds held in the escrow. I believe this creates a centralization risk.

Proof of Concept

https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/gateway/BridgeEscrow.sol#L24-L30

Tools Used

Manual review

Recommended Mitigation Steps

Consider allowing the approval only to the L1GraphTokenGateway contract.

0xean commented 1 year ago

dupe of #40