NFTBridge
60,000 USDC
View results
Submission Details
Severity: low
Invalid

The `_consumeMessageAutoWithdraw` function does not work properly

Summary

The _consumeMessageAutoWithdraw function does not work properly

Vulnerability Details

The _consumeMessageAutoWithdraw is intended to execute messages that have been checked by the owner and set them as consumed once used to prevent replay attack. The function works like this:

uint256 constant WITHDRAW_AUTO_NONE = 0x00;
uint256 constant WITHDRAW_AUTO_READY = 0x01;
uint256 constant WITHDRAW_AUTO_CONSUMED = 0x02;
function _consumeMessageAutoWithdraw(
snaddress fromL2Address,
uint256[] memory request
)
internal
{
bytes32 msgHash = keccak256(
abi.encodePacked(
snaddress.unwrap(fromL2Address),
uint256(uint160(address(this))),
request.length,
request)
);
uint256 status = _autoWithdrawn[msgHash];
if (status == WITHDRAW_AUTO_CONSUMED) {
revert WithdrawAlreadyError();
}
_autoWithdrawn[msgHash] = WITHDRAW_AUTO_CONSUMED;
}
function addMessageHashForAutoWithdraw(
uint256 msgHash
)
external
payable
onlyOwner
{
bytes32 hash = bytes32(msgHash);
if (_autoWithdrawn[hash] != WITHDRAW_AUTO_NONE) {
revert WithdrawMethodError();
}
_autoWithdrawn[hash] = WITHDRAW_AUTO_READY;
emit MessageHashAutoWithdrawAdded(hash);
}

As we can see, the default state of a message is the 0x00 (WITHDRAW_AUTO_NONE). If a message is eligible to be auto withdrawed, the contract owner will call the addMessageHashForAutoWithdraw which ensures that the message state is the default (NONE) and changes it to the READY state. Only once a message is in the READY state, can be consumed by calling the _consumeMessageAutoWithdraw internally.

However, if we look closely to the _consumeMessageAutoWithdraw we can see that it first ensures that the state of the message is not the CONSUMED and after that it sets it to CONSUMED. The issue here is that a message in the default state (NONE) will pass the check to not be CONSUMED. When in reality, the intended behavior is that a message can only be consumed once it is in READY state.

Impact

Even though this feature is currently disabled, the impact is that any message will be able to be auto withdrawn once the functionality becomes live.

Tools Used

Manual review

Recommendations

function _consumeMessageAutoWithdraw(
snaddress fromL2Address,
uint256[] memory request
)
internal
{
bytes32 msgHash = keccak256(
abi.encodePacked(
snaddress.unwrap(fromL2Address),
uint256(uint160(address(this))),
request.length,
request)
);
uint256 status = _autoWithdrawn[msgHash];
+ if (status != WITHDRAW_AUTO_READY) {
- if (status == WITHDRAW_AUTO_CONSUMED) {
revert WithdrawAlreadyError();
}
_autoWithdrawn[msgHash] = WITHDRAW_AUTO_CONSUMED;
}
Updates

Lead Judging Commences

n0kto Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!