search

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

0 stars 0 forks source link

Using ifAdmin modifier to forcefully interact with implementation contracts via _fallback() call. #275

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/upgrades/GraphProxy.sol#L21-L27 https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/upgrades/GraphProxy.sol#L170

Vulnerability details

Impact

The modifier ifAdmin() allows internal delegation to the implementation contract if caller is not admin by calling the _fallback() function which delegates the current call to implementation.

This allows a user who is not admin to call to make a transaction call with data to GraphProxy functions restricted with ifAdmin or ifAdminOrPendingImpl modifier to forcefully initialize implementation contract. and carry out activities on implementation, for example calling GraphTokenUpgradeable.permit() to approve allowance.

Proof of Concept

  1. Alice is not an admin, and sends a transaction call to GraphProxy.setAdmin() with transaction data to make GraphTokenUpgradeable.permit() call
  2. Since Alice is not an admin, the call jumps to _fallback() which makes a delegation call with the transaction data to implementation contract.
  3. _fallback() makes a delegation call to GraphTokenUpgradeable.permit() to approve Alice as spender for token owner
  4. allowance is approved for alice to spend.

Tools Used

Manual review

Recommended Mitigation Steps

The functionality of the _fallback() function may need to be revisited and/or necessarily amending the ifAdmin and ifAdminOrPendingImpl modifiers.

0xean commented 1 year ago

closing as dupe of #279