code-423n4 / 2023-11-panoptic-findings

0 stars 0 forks source link

Examining Incorrect Return Values from the add(int256,int256) Function in the LeftRight Library #617

Closed c4-bot-6 closed 11 months ago

c4-bot-6 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/types/LeftRight.sol#L159-L171

Vulnerability details

Impact

The add(int256,int256) function in the LeftRight library is designed to return the addition of two int256 numbers. However, due to incorrect implementation, it returns erroneous values in specific edge cases, resulting in precision loss.

This function is used multiple times in the SemiFungiblePositionManager contract, and its incorrect behavior can cause heavy financial losses.

Proof of Concept

This function has lot of issues:

Here is a Foundry POC, run inside test/foundry with command forge test --match-test heist -vvv

pragma solidity ^0.8.0;

import {LeftRight} from "@types/LeftRight.sol";

import "forge-std/Test.sol";

contract LRTest is Test {
using LeftRight for int256;
using LeftRight for int128;

// when either x || y < 0 ; assume both positive, if smaller number has '-' it will be bug
// lets take two examples
// -3 and 2 ; 3 and - 2
// here we will assume both are positive
// then we will verfiy if the smaller number is negative it is a bug
// in -3,2 its addition would be normal since larger number is negative
// in 3,-2 its addtion would be buggy since smaller number is negative

function test_32_case1_normal_heist() public {
 console.logInt(int256(-3).add(int256(2))); //normal
}

function test_32_case2_bug_heist() public {
 console.logInt(int256(3).add(int256(-2))); //buggy
//  returns -340282366920938463463374607431768211455
}

// bug: when x < 0 and y < 0
function test_xy_both_negative_heist() public {
 console.logInt(int256(-19).add(int256(-19)));
 console.logInt(int256(-91).add(int256(-19)));
 console.logInt(int256(-67).add(int256(-76)));

//returns
//   -340282366920938463463374607431768211494
//   -340282366920938463463374607431768211566
//   -340282366920938463463374607431768211599

}

}

This add() function also gives fake overflow on int128 max but in other case doesn't revert on int256 overflow (timeUp, couldn't write poc for this one)

Tools Used

Echidna, foundry

Recommended Mitigation Steps

Assessed type

Math

c4-judge commented 11 months ago

Picodes marked the issue as unsatisfactory: Invalid