This line re-encodes _data with the selector for the claimRewards function of the IRewardsProxy interface, which is intended to dynamically call the claimRewards function on the proxy contract with the provided _data.
By re-encoding _data, it assumes _data contains parameters for the claimRewards function, but it also wraps it again with the same function selector, which might not be necessary or correct depending on how _data was initially prepared. This could lead to incorrect encoding and hence a failed call if _data was already correctly encoded for the delegatecall.
Let's say a user wants to claim a reward with rewardId = 1. They send a transaction with _data prepared for this claim. However, due to the re-encoding issue:
The rewardsProxy receives the call and re-encodes _data, changing its intended structure.
It then makes a delegatecall to RewardsContract, passing the incorrectly formatted _data.
RewardsContract fails to decode rewardId from _data correctly because the data structure does not match the expectation.
The transaction reverts, and the user is unable to claim their reward.
Tools Used
Manual Review
Recommended Mitigation Steps
Ensure that the input data (_data) is correctly prepared for the intended function call. If _data is already encoded with arguments for the target function, additional encoding with the function selector should not be necessary.
Lines of code
https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L398
Vulnerability details
Impact
_data = abi.encodeWithSelector(IRewardsProxy(rewardsProxy).claimRewards.selector, _data);
This line re-encodes _data with the selector for the claimRewards function of the IRewardsProxy interface, which is intended to dynamically call the claimRewards function on the proxy contract with the provided _data.
By re-encoding _data, it assumes _data contains parameters for the claimRewards function, but it also wraps it again with the same function selector, which might not be necessary or correct depending on how _data was initially prepared. This could lead to incorrect encoding and hence a failed call if _data was already correctly encoded for the delegatecall.
Proof of Concept
https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L398
https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L394-L403
Let's say a user wants to claim a reward with rewardId = 1. They send a transaction with _data prepared for this claim. However, due to the re-encoding issue:
The rewardsProxy receives the call and re-encodes _data, changing its intended structure. It then makes a delegatecall to RewardsContract, passing the incorrectly formatted _data. RewardsContract fails to decode rewardId from _data correctly because the data structure does not match the expectation. The transaction reverts, and the user is unable to claim their reward.
Tools Used
Manual Review
Recommended Mitigation Steps
Ensure that the input data (_data) is correctly prepared for the intended function call. If _data is already encoded with arguments for the target function, additional encoding with the function selector should not be necessary.
Assessed type
call/delegatecall