Tapioca DAO - cryptonue's results

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Magnetar/MagnetarV2.sol#L236-L238

Vulnerability details

Impact

MagnetarV2 burst function will always revert especially when there is TOFT_WRAP action due to double accumulate the value for valAccumulator

Proof of Concept

The burst function in MagnetarV2.sol serves the purpose of batching multiple calls together with only one parameter, calls, which represents the list of actions to be performed. This function is designed to be payable, as there might be some msg.value to be sent with each action call. The contract includes a check at the end of the function to ensure that the total value of all msg.value in each call (stored in valAccumulator) matches the msg.value sent when calling the function.

File: MagnetarV2.sol
714:         require(msg.value == valAccumulator, "MagnetarV2: value mismatch");

However, a potential issue arises when examining lines 215 and 237, where valAccumulator can be called twice when the action ID is TOFT_WRAP. This double calculation of valAccumulator will inevitably lead to an incorrect value. Consequently, the subsequent require check will fail due to the mismatched values.

To address this issue and ensure accurate calculations, it is essential to modify the code to account for the possibility of valAccumulator being called twice in cases where the action ID is TOFT_WRAP. One possible solution could involve carefully tracking and updating the valAccumulator variable to avoid double-counting msg.value in such scenarios. By addressing this bug, the burst function will function correctly and maintain the integrity of its value checks, providing a secure and reliable batching mechanism for multiple actions within the MagnetarV2 contract.

File: MagnetarV2.sol
201:
202:         for (uint256 i = 0; i < length; i++) {
203:             Call calldata _action = calls[i];
...
214:             unchecked {
215:                 valAccumulator += _action.value;
216:             }
217:
218:             if (_action.id == PERMIT_ALL) {
...
225:             } else if (_action.id == PERMIT) {
...
232:             } else if (_action.id == TOFT_WRAP) {
233:                 WrapData memory data = abi.decode(_action.call[4:], (WrapData));
234:                 _checkSender(data.from);
235:                 if (_action.value > 0) {
236:                     unchecked {
237:                         valAccumulator += _action.value;
238:                     }
...
248:                 }
249:             } else if (_action.id == TOFT_SEND_FROM) {
...
...:             } else if (_action.id == ...) {
...
709:             } else {
710:                 revert("MagnetarV2: action not valid");
711:             }
712:         }
713:
714:         require(msg.value == valAccumulator, "MagnetarV2: value mismatch");

Tools Used

Manual analysis

Recommended Mitigation Steps

Remove the addition of valAccumulator on TOFT_WRAP

Assessed type

Payable