Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

`Treasury::allocateFunds` Can Be Front Runned

pacelli

Summary

This function is used by the account with the ALLOCATOR_ROLE to set an allowance to an account but given the current implementation the function can be front runned.

function allocateFunds(address recipient, uint256 amount) external override onlyRole(ALLOCATOR_ROLE) {
if (recipient == address(0)) revert InvalidRecipient();
if (amount == 0) revert InvalidAmount();
​
_allocations[msg.sender][recipient] = amount;
emit FundsAllocated(recipient, amount);
}

Vulnerability Details

The vulnerable line in the function is:

_allocations[msg.sender][recipient] = amount;

This line of code only replaces the old allocation with a new one and it can be exploited in the following scenario:

  1. The spender has set up a listener to subscribe to new transaction sent to the mempool

  2. The allocator decides to reduce the allowance and submits the transaction

  3. The bot of the spender scan the new transaction and decodes the data field to determine if it is a transaction to reduce the allocation

  4. If it is, it front runs the transaction by simply sending a transaction with a higher maxPriorityFeePerGas to get his transaction processed first

  5. Both transaction are picked up and included in a block

Let's say Bob gave an allocation of 100 tokens to Alice and later decided to reduce it to 50. Alice performs the attack detailed above and spend the 100 tokens of the original allowance and get a new allowance of 50. This give the possibility to Alice to access 150 tokens of Bob.

Impact

I was unable to determine where in the protocol the allocated funds can be spent or withdrawn, but it clear this is the intend of the function, for that reason I determined the impact to be: transaction front runned the attacker can spent more tokens with a HIGH severity.

Tools Used

Manual review

Recommendations

I recommend implementing a mechanism like OpenZeppelin's increaseAllowance and decreaseAllowance to protect against frontrunning, these functions instead of overwriting the old allowance they increase or reduce the allocation by an amount. Aossible implementation of these function could be:

function increaseAllocatedFunds(address recipient, uint256 amount) external onlyRole(ALLOCATOR_ROLE) {
if (recipient == address(0)) revert InvalidRecipient();
if (amount == 0) revert InvalidAmount();
​
_allocations[msg.sender][recipient] += amount;
emit FundsAllocated(recipient, amount);
}
​
function decreaseAllocatedFunds(address recipient, uint256 amount) external onlyRole(ALLOCATOR_ROLE) {
if (recipient == address(0)) revert InvalidRecipient();
if (amount == 0) revert InvalidAmount();
​
uint256 currentAllocation = _allocations[msg.sender][recipient];
// If the current allocation is less than the amount to reduce by
// then it means the spender front runned the transaction and is
// trying to access to more funds. If it's the case revert.
if(currentAllocation < amount) {
revert AllowanceAlreadySpend();
}
​
_allocations[msg.sender][recipient] -= amount;
emit FundsAllocated(recipient, amount);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Treasury::allocateFunds should increase or decrease funds to avoid recipient frontrunning and double spending

Support

FAQs

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