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

`LibStrings.toString(int256 value)` does not handle all `int256` values correctly.

tricko

Summary

LibStrings.toString(int256 value) cannot handle value = type(int256).min or value = 0.

Vulnerability Details

LibStrings.toString(int256 value) does not handle all int256 values correctly.

function toString(int256 value) internal pure returns(string memory){
if(value > 0){
return toString(uint256(value));
} else {
return string(abi.encodePacked("-", toString(uint256(-value))));
}
}

The incorrectly handled values are type(int256).min and 0:

  • toString(int256 value) method handles negative int256 values by first converting them to positive, then casting them to uint256, and passing them to LibStrings.toString(uint256 value). However, if value = type(int256).min, the negation -value will cause an overflow because -type(int256).min exceeds type(int256).max, due to the asymmetry of int256. Consequently the call to toString(int256 value) will revert.

  • value = 0 is catched by the else branch and thus will generate incorrectly formatted negative strings like -0.

See the example POC for the value = type(int256).min case below (Run it using foundry):

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
​
import {Test, console} from "forge-std/Test.sol";
​
contract LibString {
//Empty implementation as it is not needed for the POC
function toString(uint256 value) public returns(string memory) {}
​
function toString(int256 value) public returns(string memory) {
if(value > 0){
return toString(uint256(value));
} else {
return string(abi.encodePacked("-", toString(uint256(-value))));
}
}
}
​
contract POC is Test {
LibString public libstring;
​
function setUp() public {
libstring = new LibString();
}
​
function testParse() public {
int256 value = type(int256).min;
vm.expectRevert();
libstring.toString(value);
}
}

Impact

As described above, LibStrings.toString(int256 value) does not handle correctly value = type(int256).min or value = 0, which affects the functionality of MetadataFacet.sol. Nevertheless, since both are edge cases, it should be considered a low severity issue.

Tools Used

Manual Review.

Recommendations

Consider applying the fix below.

diff --git a/LibStrings.sol b/LibStrings.mod.sol
index 64f7363..ad5aa41 100644
--- a/LibStrings.sol
+++ b/LibStrings.mod.sol
@@ -56,10 +56,13 @@ library LibStrings {
* @dev Converts a `int256` to its ASCII `string` representation.
*/
function toString(int256 value) internal pure returns(string memory){
- if(value > 0){
+ if(value >= 0){
return toString(uint256(value));
- } else {
+ } else if (value != type(int256).min) {
return string(abi.encodePacked("-", toString(uint256(-value))));
+ } else {
+ uint256 modValue = uint256(value + 1) + 1;
+ return string(abi.encodePacked("-", toString(modValue)));
}
}
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.