Beanstalk Part 3
Beanstalk
DeFiHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

`LibString.toString(uint256)` will revert for all values besides 0.

tricko

Summary

Due to incorrect code logic in LibString.toString(uint256), index-- will cause an arithmetic underflow during the last iteration, making the function revert for all values except 0.

Vulnerability Details

Consider the LibString.toString(uint256), shown below:

function toString(uint256 value) internal pure returns (string memory) {
if (value == 0) {
return "0";
}
uint256 temp = value;
uint256 digits;
while (temp != 0) {
digits++;
temp /= 10;
}
bytes memory buffer = new bytes(digits);
uint256 index = digits - 1;
temp = value;
while (temp != 0) { // << problematic loop
buffer[index--] = bytes1(uint8(48 + temp % 10));
temp /= 10;
}
return string(buffer);
}

When processing any value other than zero, during the last iteration of the second while loop, index will be 0. Therefore, decrementing index (index--) will result in an arithmetic underflow, causing the function to revert. This will occur for every value passed as an argument, except zero.

See the POC below (Run using foundry).

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
​
import {Test, console} from "forge-std/Test.sol";
​
contract LibString {
function toString(uint256 value) public pure returns (string memory) {
if (value == 0) {
return "0";
}
uint256 temp = value;
uint256 digits;
while (temp != 0) {
digits++;
temp /= 10;
}
bytes memory buffer = new bytes(digits);
uint256 index = digits - 1;
temp = value;
while (temp != 0) {
buffer[index--] = bytes1(uint8(48 + temp % 10));
temp /= 10;
}
return string(buffer);
}
}
​
contract POC is Test {
LibString public libstring;
​
function setUp() public {
libstring = new LibString();
}
​
function testRevert() public {
uint256 value = 2024;
vm.expectRevert();
libstring.toString(value);
}
}

Impact

Because of the underflow described above, LibString.toString(uint256) will revert for all values except 0. Since LibString.toString(uint256) is widely used in MetadataFacet.sol and MetadataImage.sol, it will be impossible to retrieve the URI or imageURI.

Tools Used

Manual Review.

Recommendations

Consider not decreasing index in the last iteration of the second while loop. As shown below:

diff --git a/LibStrings.sol b/LibStrings.mod.sol
index 64f7363..2dc6c68 100644
--- a/LibStrings.sol
+++ b/LibStrings.mod.sol
@@ -30,7 +30,10 @@ library LibStrings {
uint256 index = digits - 1;
temp = value;
while (temp != 0) {
- buffer[index--] = bytes1(uint8(48 + temp % 10));
+ buffer[index] = bytes1(uint8(48 + temp % 10));
+ if (index > 0) {
+ index = index - 1;
+ }
temp /= 10;
}
return string(buffer);
Updates

Lead Judging Commences

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

Support

FAQs

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