Back to a_delamo's contests

FairSide contest - a_delamo's results

FairSide Network

General Information

Platform: Code4rena

Start Date: 20/05/2021

Pot Size: $55,000 USDC

Total HM: 19

Participants: 8

Period: 7 days

Judge: cemozer

Total Solo HM: 11

Id: 11

League: ETH

FairSide

Findings Distribution

Researcher Performance

Rank: 4/8

Findings: 4

Award: $3,011.94

🌟 Selected for report: 4

🚀 Solo Findings: 1

External Links

Code4rena Contest PageGithub Contest RepositoryGithub Findings RepositoryFairSide

Findings Information

🌟 Selected for report: shw

Also found by: a_delamo, cmichel, pauliax

Labels

bug
duplicate
2 (Med Risk)

Awards

1441.1236 USDC - $1,441.12

External Links

Link to GitHub issue

Handle

a_delamo

Vulnerability details

Impact

In Withdrawable.sol, every time a user wants to withdraw, the following code will get executed:

    function _increaseWithdrawal(address user, uint256 amount) internal {
        availableWithdrawal[user] = availableWithdrawal[user].add(amount);
        pendingWithdrawals = pendingWithdrawals.add(amount);
    }

Then the user will call need to call the withdraw method, to retrieve the amount he wanted to withdraw.

function withdraw() external {
        uint256 reserveAmount = availableWithdrawal[msg.sender];
        require(reserveAmount > 0, "FSD::withdraw: Insufficient Withdrawal");
        delete availableWithdrawal[msg.sender];
        msg.sender.transfer(reserveAmount);
    }

The issue is that while _increaseWithdrawal, increase the pendingWithdrawals state variable, withdraw does not reduce this property. Meaning pendingWithdrawals will only increase, affecting the getReserveBalance method

function getReserveBalance() public view returns (uint256) {
        return address(this).balance.sub(pendingWithdrawals);
    }

This method is really important because is being used to calculate the number of tokens to burn/mint and to calculate the price of the tokens using the ABC.sol

 function getFSDPrice() public view returns (uint256) {
        // FSHARE = Total Available Cost Share Benefits / Gearing Factor
        uint256 fShare = totalCostShareBenefits / GEARING_FACTOR;
        // Floor of 7500 ETH
        if (fShare < 7500 ether) fShare = 7500 ether;

        // Capital Pool = Total Funds held in ETH – Open Cost Share Requests
        // Open Cost Share Request = Cost share request awaiting assessor consensus
        uint256 capitalPool = fsd.getReserveBalance() - totalOpenRequests;

        return FairSideFormula.f(capitalPool, fShare);
    }

#0 - fairside-core

2021-05-30T13:07:06Z

Duplicate of #72, should be increased in severity.

Findings Information

🌟 Selected for report: a_delamo

Labels

bug
2 (Med Risk)
sponsor acknowledged

Awards

1441.1236 USDC - $1,441.12

External Links

Link to GitHub issue

Handle

a_delamo

Vulnerability details

Impact

FairSideFormula library is using ABDKMathQuad library underneath. According to the ABDKMathQuad README, the range of values is the following:

The minimum strictly positive (subnormal) value is 2^−16494 ≈ 10^−4965 and has a precision of only one bit. The minimum positive normal value is 2^−16382 ≈ 3.3621 × 10^−4932 and has a precision of 113 bits, i.e. ±2^−16494 as well. The maximum representable value is 2^16384 − 2^16271 ≈ 1.1897 × 10^4932.

Using Echidna, a fuzzing tool for smart contracts, I found some edge cases when some of the operations do not work as expected. This is the test code I run using echidna-test contracts/TestABDKMathQuad --contract TestABDKMathQuad

// SPDX-License-Identifier: Unlicense
pragma solidity >=0.6.0 <0.8.0;

import "./dependencies/ABDKMathQuad.sol";
import "@openzeppelin/contracts/math/SafeMath.sol";

contract TestABDKMathQuad {
    uint256 internal x;
    uint256 internal x1;

    int256 internal y;

    function setX(uint256 _x) public {
        x = _x;
    }

    function setX1(uint256 _x1) public {
        x1 = _x1;
    }

    function setY(int256 _y) public {
        y = _y;
    }

    function echidna_Uint_convertion() public returns (bool) {
        bytes16 z = ABDKMathQuad.fromUInt(x);
        return ABDKMathQuad.toUInt(z) == x;
    }

    function echidna_int_convertion() public returns (bool) {
        bytes16 z = ABDKMathQuad.fromInt(y);
        return ABDKMathQuad.toInt(z) == y;
    }

    function echidna_mulUint() public returns (bool) {
        uint256 mul = SafeMath.mul(x, x1);

        bytes16 z = ABDKMathQuad.fromUInt(x);
        bytes16 z1 = ABDKMathQuad.fromUInt(x1);
        bytes16 t = ABDKMathQuad.mul(z, z1);
        return ABDKMathQuad.toUInt(t) == mul;
    }

    function echidna_divUint() public returns (bool) {
        if (x1 == 0 || x == 0 || x < x1) return true;
        uint256 div = SafeMath.div(x, x1);

        bytes16 z = ABDKMathQuad.fromUInt(x);
        bytes16 z1 = ABDKMathQuad.fromUInt(x1);
        bytes16 t = ABDKMathQuad.div(z, z1);
        return ABDKMathQuad.toUInt(t) == div;
    }

    function echidna_neg() public returns (bool) {
        bytes16 z = ABDKMathQuad.fromInt(y);
        bytes16 z_positive = ABDKMathQuad.neg(z);
        int256 result = ABDKMathQuad.toInt(z_positive);

        return result == (-y);
    }

    function echidna_ln() public returns (bool) {
        if (x == 0) return true;

        bytes16 z = ABDKMathQuad.fromUInt(x);
        bytes16 result = ABDKMathQuad.ln(z);
        if (result == ABDKMathQuad.NaN) return false;
        uint256 result_uint = ABDKMathQuad.toUInt(result);
        return result_uint < x;
    }
}

And the results are:

echidna_mulUint: failed!💥
  Call sequence:
    setX(1)
    setX1(10389074519043615041642520862277205)

echidna_Uint_convertion: failed!💥
  Call sequence:
    setX(10385528305364854446597578558364193)

echidna_neg: failed!💥
  Call sequence:
    setY(-10394149475425461937254292332080605)

echidna_int_convertion: failed!💥
  Call sequence:
    setY(-10418479581230103876421151985443129)

echidna_divUint: failed!💥
  Call sequence:
    setX1(1)
    setX(10518626300707317802075092650125337)

If we check in Remix, we can see there is a small difference when converting from UInt to Bytes16 or the opposite way. This is probably the same issue with all the other operations.

ABDKMathQuad.fromUInt(10385528305364854446597578558364193);
// 0x40700005e5e8a8c4dcb4999a17cead10
ABDKMathQuad.toUInt(0x40700005e5e8a8c4dcb4999a17cead10)
//10385528305364854446597578558364192

Tools Used

Echidna https://github.com/crytic/echidna

Use some fuzzing tool like Echidna to verify there is no edge cases

#0 - fairside-core

2021-05-30T15:09:25Z

I am slightly mixed about this finding. We did employ fuzz tests during the audit we had gone through and they were unable to pinpoint any issues in the value range we expect the curve to be utilized in. This is definitely a good suggestion and one we will assimilate, however, I am not sure how one would judge the severity of this.

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter