Open code423n4 opened 2 years ago
WatchPug
https://github.com/livepeer/arbitrum-lpt-bridge/blob/ebf68d11879c2798c5ec0735411b08d0bea4f287/contracts/L1/escrow/L1Escrow.sol#L21-L28
function approve( address _token, address _spender, uint256 _value ) public onlyRole(DEFAULT_ADMIN_ROLE) { ApproveLike(_token).approve(_spender, _value); emit Approve(_token, _spender, _value); }
L1Escrow.sol#approve() allows an address with DEFAULT_ADMIN_ROLE can approve an arbitrary amount of tokens to any address.
L1Escrow.sol#approve()
DEFAULT_ADMIN_ROLE
We believe this is unnecessary and poses a serious centralization risk.
A malicious or compromised DEFAULT_ADMIN_ROLE address can take advantage of this, and steal all the funds from the L1Escrow contract.
L1Escrow
Consider removing approve() function and approve l1LPT to l1Gateway in the constructor.
approve()
l1LPT
l1Gateway
Likely won't change as we want to preserve the ability for protocol governance to move the LPT from the L1Escrow in the event of a L2 failure.
duplicate of #165 ?
Handle
WatchPug
Vulnerability details
https://github.com/livepeer/arbitrum-lpt-bridge/blob/ebf68d11879c2798c5ec0735411b08d0bea4f287/contracts/L1/escrow/L1Escrow.sol#L21-L28
L1Escrow.sol#approve()
allows an address withDEFAULT_ADMIN_ROLE
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
DEFAULT_ADMIN_ROLE
address can take advantage of this, and steal all the funds from theL1Escrow
contract.Recommendation
Consider removing
approve()
function and approvel1LPT
tol1Gateway
in the constructor.