sherlock-audit / 2023-10-aloe-judging

9 stars 6 forks source link

drextron - Bit-bleeding in slot-stuffed variables can result in messed up variable values that can result in unintended consequences for the protocol #98

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

drextron

medium

Bit-bleeding in slot-stuffed variables can result in messed up variable values that can result in unintended consequences for the protocol


name: Audit item #2 about: Bit-bleeding in slot-stuffed variables can result in messed up variable values that can result in unintended consequences for the protocol title: "Bit-bleeding" labels: "Medium severity" assignees: ""

Summary

Throughout the codebase, slot packing is used. To avoid wasting space and gas, Aloe (cleverly) packs many values into one uint256 slot. This is smart for many reasons, but also opens the protocol to many possible vulnerabilities. I will highlight one in this issue, however this vulnerability applies in multiple parts of the code base wherever a single uint256 variables packs bits of multiple variables in lieu of using a struct instead.

In the Lender.sol contract, the balances array holds data per address on a user's balances. From the inline comments, for example in _mint [https://github.com/sherlock-audit/2023-10-aloe/blob/main/aloe-ii/core/src/Lender.sol#L439]:

From most to least significant [bit]... courier id | 32 bits user's principle | 112 bits user's balance | 112 bits

112 bits should be enough to keep track of both principle and balance for any feasible, real world position/balance. However, is enough couriers for a given user are created in the referral program logic, it's possible to construct an attack to make the courier portion of this data slot in balances bleed into the "user's principle" portion

Vulnerability Detail

In the way in which new couriers are created and tied to some beneficiary/user address, if more couriers than can be counted in 32 bits could be created for a given address, then the courier bits will bleed into the "user's principle" bits.

The maximum value that could be represented in 32 bits is 2^32 - 1, which is equal to 4,294,967,295. Though that's a high number, if Aloe functions on a chain that at some point now or in the future has low enough transaction costs (assuming you can create one new courier per transaction) that it's feasible to create more than 4,294,967,295 couriers for a single address, then the "user's principle" bits will be bled into and poisoned. This can result in a calamity for Aloe. Depending on how many bits of this portion are bled into, the "user's principle" section of 112 bits can be manipulated, underflown, or overflown.

For example, the average transaction cost on Solana chain (I am aware that Aloe is not currently deployed to this chain, but it may be someday. Just to illustrate my point with exact values from a mainnet chain that currently have low enough tx costs to exploit this vulnerability case) as of the time of this writing is roughly $0.0002 [https://coincodex.com/article/24933/solana-gas-fees/]. In USD value, it would cost an attacker $859,493 to create 4,294,967,295 couriers for a single address, assuming this is done in 4,294,967,295 separate transactions (can be less if can be done in batched tx's). While roughly $860k isn't necessarily cheap by any means, if an attacker has a crafty enough attack that involves a large enough financial incentive to carry out (i.e. the balances "user's principle" data value for a given address that the attackers intends to manipulate could be manipulated to decrease this value on a HUGE loan amount (say, much much larger than $860k in this case) by so much, that you could effectively "pay down the principle" by simply carefully manipulating the 112 bits of "user's principle" in a way that benefits the attacker to the extent where the $860k in fees is worth it), such a targeted bit-bleed attack on an address' balance, then spending $860k might be worth the trouble.

Over time, with the rise of both L1 scaling initiatives and widespread rollup/L2 adoption, we can expect tx fees to continue dropping. Via https://dune.com/haddis3/optimism-fee-calculator, Optimism (which Aloe is currently deployed on) already has gas prices as low as 0.001 Gwei. These fees/gas prices will only go lower, and thus make bit-bleed attacks on Aloe protocol that much more affordable and feasible.

Additionally, another case of this type of potential "bit-bleed vulnerability" can be found in Borrower.sol wherever slot0 variable is used, where positions, unleashLiquidationTime, state, and dirt variables are all stuffed into one slot. Some of these cases are easier to exploit than others, however the big problem here is that in any of these cases, **if an attacker can achieve an attack where a variable's bits are bled into other variables' range of bits, then Aloe protocols' entire accounting system can start to be riddled with inaccurate values all over the place that could make the whole system's sanctity, reliable, accuracy, and user trust at risk.

Impact

If the "user's principle" bits are manipulated, underflown, or overflown, this can result in inaccurate accounting figures for a user address. This can result in incorrect principle for this address, or specifically in the case of underflow and/or overflow, a user's principle can be zeroed out or be inflated to a number magnitudes higher than it really is. I don't think I need to explain what a tragedy this could result in for Aloe at a risk/accounting standpoint.

Code Snippet

Tool used

Solidity

Recommendation

Either create a struct to use for tracking courier id, user's principle, and user's balance values (I know you're trying to save both space and gas by slot packing, but is it worth these potential risks?) or add logic to enforce explicit max/min values that are slot-packed.

sherlock-admin2 commented 1 year ago

3 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, because 32-bit courier id amount can't bleed into the other bits, there is no POC provided or a valid scenario describing how the bits can bleed.

tsvetanovv commented:

This is design decision

MohammedRizwan commented:

seems intended design