Platform: Code4rena
Start Date: 07/10/2022
Pot Size: $50,000 USDC
Total HM: 4
Participants: 62
Period: 5 days
Judge: 0xean
Total Solo HM: 2
Id: 169
League: ETH
Rank: 5/62
Findings: 2
Award: $1,803.21
🌟 Selected for report: 0
🚀 Solo Findings: 0
1752.9293 USDC - $1,752.93
Similar to https://github.com/code-423n4/2022-01-livepeer-findings/issues/195
BridgeEscrow.sol#approveAll() allows Governor can approve an arbitrary amount of tokens to any address.
We believe this is unnecessary and poses a serious centralization risk.
A malicious or compromised Governor address can take advantage of this, and steal all the funds from the BridgeEscrow contract.
None
Consider removing approveAll() function and approve graphToken to L1GraphTokenGateway in the constructor.
#0 - 0xean
2022-10-15T23:29:37Z
This is the intended design to allow for multiple bridges in the future. Going to leave open for sponsor review, but may decide to close as invalid.
#1 - pcarranzav
2022-10-18T18:38:12Z
Yes, this is intentional to allow multiple bridges but also to allow for recovery of the funds in case of a critical issue in Arbitrum.
Note that the Governor is not a regular EOA, but a Gnosis Safe managed by The Graph Council; this account is already able to control many protocol parameters, including adding minters to the GRT contract, so adding this control over the escrow does not change the trust assumption significantly imo.
#2 - 0xean
2022-10-21T21:53:35Z
@pcarranzav - totally understand that. I personally would happily close this issue as invalid, but Code4rena seems to have a pattern of awarding these as M to ensure that users are aware of the risk, even as low as it may be.
#3 - 0xean
2022-10-21T22:16:05Z
going to use #300 as primary.
🌟 Selected for report: 0xSmartContract
Also found by: 0x1f8b, 0x4non, 0xNazgul, Bnke0x0, Chom, IllIllI, Josiah, Rahoz, RaymondFam, Trust, Waze, ajtra, bobirichman, brgltd, bulej93, c3phas, cccz, chrisdior4, delfin454000, fatherOfBlocks, gogo, ladboy233, mcwildy, mics, nicobevi, oyc_109, rbserver, rotcivegaf, zzzitron
50.2765 USDC - $50.28
Version 3.4.2 of the openzeppelin/contracts-upgradeable library is used in the code. According to https://security.snyk.io/package/npm/@openzeppelin%2Fcontracts-upgradeable/3.4.2, the following two vulnerabilities are possible: In scope: https://security.snyk.io/vuln/SNYK-JS-OPENZEPPELINCONTRACTSUPGRADEABLE-2980280
Not in scope:
https://security.snyk.io/vuln/SNYK-JS-OPENZEPPELINCONTRACTSUPGRADEABLE-2320177
https://security.snyk.io/vuln/SNYK-JS-OPENZEPPELINCONTRACTSUPGRADEABLE-2980280
https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/disputes/DisputeManager.sol#L745-L749 https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/staking/Staking.sol#L1117-L1118 https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/statechannels/AllocationExchange.sol#L158-L159 https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/token/GraphToken.sol#L116-L117
None
Change to version 4.7.3
#0 - 0xean
2022-10-14T19:02:48Z
Downgrading to QA.
#1 - pcarranzav
2022-10-19T17:25:27Z
We can't upgrade because we're using solidity 0.7 (and newer OZ versions only support 0.8)