DeFiHardhat
21,000 USDC
View results
Submission Details
Severity: medium
Invalid

`Libsilo::transferStalk` has rounding and off-by-one error

Summary

`/**
 * @notice Decrements the Stalk and Roots of `sender` and increments the Stalk
 * and Roots of `recipient` by the same amount.
 *
 */
function transferStalk(address sender, address recipient, uint256 stalk) internal {
    AppStorage storage s = LibAppStorage.diamondStorage();
    uint256 roots;
    roots = stalk == s.a[sender].s.stalk
        ? s.a[sender].roots
        : s.s.roots.sub(1).mul(stalk).div(s.s.stalk).add(1);


    // Subtract Stalk and Roots from the 'sender' balance.
    s.a[sender].s.stalk = s.a[sender].s.stalk.sub(stalk);
    s.a[sender].roots = s.a[sender].roots.sub(roots);
    emit StalkBalanceChanged(sender, -int256(stalk), -int256(roots));


    // Add Stalk and Roots to the 'recipient' balance.
    s.a[recipient].s.stalk = s.a[recipient].s.stalk.add(stalk);
    s.a[recipient].roots = s.a[recipient].roots.add(roots);
    emit StalkBalanceChanged(recipient, int256(stalk), int256(roots));
}`

Vulnerability Details

Scenario 1: Partial Stalk Transfer
Total stalk in the system (s.s.stalk) = 1000
Total roots in the system (s.s.roots) = 500
Sender's stalk (s.a[sender].s.stalk) = 100
stalk being transferred = 50
Expected Outcome: Proportionally, 50 roots should be transferred.
Calculation
roots = (500 - 1) * 50 / 1000 + 1 = 24.95 + 1 = 25.95 ≈ 25
Issue: The calculation rounds down due to integer division, and the arbitrary .add(1) does not always correct this rounding error adequately.

Scenario 2: Edge Case with Minimum Roots
Total stalk in the system (s.s.stalk) = 1000
Total roots in the system (s.s.roots) = 1
Sender's stalk (s.a[sender].s.stalk) = 100
stalk being transferred = 50
Expected Outcome: Proportionally, 0.05 roots should be transferred, but since roots are likely integers, it should round to 0 or 1.
Calculation:
roots = (1 - 1) * 50 / 1000 + 1 = 0 + 1 = 1
Issue: The calculation incorrectly transfers 1 root due to the arbitrary addition, which is not proportional and could be unfair or incorrect depending on the intended logic of the system.

`// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "../src/Silo.sol"; // Adjust the import path as necessary

contract SiloTest is Test {
Silo public silo;
address sender = address(0x1);
address receiver = address(0x2);

function setUp() public {
    silo = new Silo();
    // Setup initial stalk and roots for the sender
    silo.setStalkAndRoots(sender, 1000, 500); // Assuming such a function exists for setup
}

function testTransferAllStalk() public {
    uint256 senderInitialRoots = silo.getRoots(sender); // Assuming getRoots function exists
    silo.transferStalk(sender, receiver, 1000); // Transfer all stalk

    uint256 senderFinalRoots = silo.getRoots(sender);
    uint256 receiverFinalRoots = silo.getRoots(receiver);

    assertEq(senderFinalRoots, 0, "Sender should have 0 roots left");
    assertEq(receiverFinalRoots, senderInitialRoots, "Receiver should have all of sender's roots");
}

function testTransferPartialStalk() public {
    uint256 transferStalkAmount = 500;
    uint256 expectedTransferRoots = (silo.getRoots(sender) * transferStalkAmount) / silo.getStalk(sender);

    silo.transferStalk(sender, receiver, transferStalkAmount);

    uint256 actualTransferRoots = silo.getRoots(receiver);
    assertEq(actualTransferRoots, expectedTransferRoots, "Roots transferred should match expected proportion");
}

function testTransferWithMinimalRoots() public {
    // Setup minimal roots scenario
    silo.setStalkAndRoots(sender, 1000, 1);
    silo.transferStalk(sender, receiver, 500);

    uint256 receiverRoots = silo.getRoots(receiver);
    assertEq(receiverRoots, 0, "Receiver should receive 0 roots due to minimal initial roots");
}

}`

Impact

The calculation of roots to be transferred along with stalk is conditional and might lead to inconsistent or incorrect results.

Tools Used

Manual Review

Recommendations

+ roots = s.s.roots.mul(stalk).div(s.s.stalk);

Updates

Lead Judging Commences

giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Out of scope

Support

FAQs

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