Beanstalk: Dive Into Basin
Beanstalk
DeFiHardhat
12,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Precision loss in `storeLastReserves()` due to truncation by intermediate division

tigerfrake

Summary

In storeLastReserves(), precision loss stems in calculation of iByte which involves an intermediate division before multiplication.

Vulnerability Details

function storeLastReserves(bytes32 slot, uint40 lastTimestamp, bytes16[] memory reserves) internal {
// Potential optimization – shift reserve bytes left to perserve extra decimal precision.
uint8 n = uint8(reserves.length);
if (n == 1) {
assembly {
sstore(slot, or(or(shl(208, lastTimestamp), shl(248, n)), shl(104, shr(152, mload(add(reserves, 32))))))
}
return;
}
assembly {
sstore(
slot,
or(
or(shl(208, lastTimestamp), shl(248, n)),
or(shl(104, shr(152, mload(add(reserves, 32)))), shr(152, mload(add(reserves, 64))))
)
)
// slot := add(slot, 32)
}
if (n > 2) {
uint256 maxI = n / 2; // number of fully-packed slots
uint256 iByte; // byte offset of the current reserve
for (uint256 i = 1; i < maxI; ++i) {
iByte = i * 64; // @audit Issue here
assembly {
sstore(
add(slot, i),
add(mload(add(reserves, add(iByte, 32))), shr(128, mload(add(reserves, add(iByte, 64)))))
)
}
}
// If there is an odd number of reserves, create a slot with the last reserve
// Since `i < maxI` above, the next byte offset `maxI * 64`
// Equivalent to "reserves.length % 2 == 1" but cheaper.
if (reserves.length & 1 == 1) {
iByte = maxI * 64; @audit Issue here
assembly {
sstore(
add(slot, maxI),
add(mload(add(reserves, add(iByte, 32))), shr(128, shl(128, sload(add(slot, maxI)))))
)
}
}
}
}

Here, if n>2, maxI is given by n / 2.

if (n > 2) {
uint256 maxI = n / 2; // number of fully-packed slots

Then in calculation of iByte, this value is multiplied by 64.

iByte = maxI * 64;

Due to truncation due to the division, the final value obtained is much smaller.

PoC

  • For n>2, take n=3

  • Calculating maxI gives n/2 =3/2 =1.5=1(truncated)

  • Calculating iByte gives maxI * 64 = 1*64 = 64

Now performing multiplication before division yields:

  • iByte = n*64/2 = 3*64/2 = 96

This is more accurate than the initial result.

Impact

Precision loss due to intermediate division before multiplication.

Tools Used

Manual Review

Recommendations

Perform multiplication before division.

function storeLastReserves(bytes32 slot, uint40 lastTimestamp, bytes16[] memory reserves) internal {
// Potential optimization – shift reserve bytes left to perserve extra decimal precision.
uint8 n = uint8(reserves.length);
if (n == 1) {
assembly {
sstore(slot, or(or(shl(208, lastTimestamp), shl(248, n)), shl(104, shr(152, mload(add(reserves, 32))))))
}
return;
}
assembly {
sstore(
slot,
or(
or(shl(208, lastTimestamp), shl(248, n)),
or(shl(104, shr(152, mload(add(reserves, 32)))), shr(152, mload(add(reserves, 64))))
)
)
// slot := add(slot, 32)
}
if (n > 2) {
uint256 maxI = n / 2; // number of fully-packed slots
uint256 iByte; // byte offset of the current reserve
for (uint256 i = 1; i < maxI; ++i) {
iByte = n*64/2; // @audit Corrected
assembly {
sstore(
add(slot, i),
add(mload(add(reserves, add(iByte, 32))), shr(128, mload(add(reserves, add(iByte, 64)))))
)
}
}
// If there is an odd number of reserves, create a slot with the last reserve
// Since `i < maxI` above, the next byte offset `maxI * 64`
// Equivalent to "reserves.length % 2 == 1" but cheaper.
if (reserves.length & 1 == 1) {
iByte = n*64/2; // @audit Corrected
assembly {
sstore(
add(slot, maxI),
add(mload(add(reserves, add(iByte, 32))), shr(128, shl(128, sload(add(slot, maxI)))))
)
}
}
}
}
Updates

Lead Judging Commences

giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

Informational/Invalid

Support

FAQs

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