Back to B2's contests

Debt DAO contest - B2's results

A cryptonative credit marketplace for fully anon and trustless loans to DAOs.

General Information

Platform: Code4rena

Start Date: 03/11/2022

Pot Size: $115,500 USDC

Total HM: 17

Participants: 120

Period: 7 days

Judge: LSDan

Total Solo HM: 1

Id: 174

League: ETH

Debt DAO

Findings Distribution

Researcher Performance

Rank: 61/120

Findings: 2

Award: $110.58

QA:
grade-b
Gas:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest PageGithub Contest RepositoryGithub Findings RepositoryDebt DAO

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0x1f8b, 0xNazgul, 0xRoxas, 0xSmartContract, Awesome, Aymen0909, B2, BClabs, Bnke0x0, Deekshith99, Deivitto, Diana, Dinesh11G, Funen, HE1M, HardlyCodeMan, Josiah, Nyx, Rahoz, RaymondFam, RedOneN, ReyAdmirado, Rolezn, Saintcode_, TomJ, Trust, __141345__, a12jmx, adriro, ajtra, aphak5010, apostle0x01, brgltd, btk, bulej93, c3phas, carlitox477, catwhiskeys, ch0bu, chaduke, chrisdior4, cryptonue, cryptostellar5, csanuragjain, ctf_sec, delfin454000, djxploit, durianSausage, erictee, fatherOfBlocks, gogo, i_got_hacked, immeas, joestakey, jumpdest7d, lukris02, martin, mcwildy, merlin, minhquanym, oyc_109, pashov, peanuts, pedr02b2, rbserver, rotcivegaf, rvierdiiev, sakman, saneryee, seyni, shark, slowmoses, tnevler, trustindistrust, w0Lfrum, yurahod, zaskoh

Awards

61.3462 USDC - $61.35

Labels

bug
QA (Quality Assurance)
grade-b
Q-59

External Links

Link to GitHub issue

Missing checks for address(0x0) when assigning values to address state variables

Missing checks for zero-addresses may lead to infunctional protocol, if the variable addresses are updated incorrectly.

        registry = FeedRegistryInterface(_registry);

File: contracts/modules/oracle/Oracle.sol (line 16)

        registry = FeedRegistryInterface(_registry);

File: contracts/modules/factories/LineFactory.sol (line 36)

Other instances of this issue are :

Unused receive() function will lock Ether in contract

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert

    receive() external payable {
        return;
    }

File: contracts/modules/spigot/Spigot.sol (line 227-229)

       receive() external payable {}

File: contracts/modules/credit/SpigotedLine.sol (line 272)

Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions

While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.

	contract Spigot is ISpigot, ReentrancyGuard {

File: contracts/modules/spigot/Spigot.sol (line 16)

	contract LineOfCredit is ILineOfCredit, MutualConsent {

File: contracts/modules/credit/LineOfCredit.sol (line 16)

Other instances of this issue are :

open TODO comments

Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment.

        // TODO: test
  • File: contracts/modules/factories/LineFactory.sol (line 140)
        // TODO: test
  • File: contracts/modules/factories/LineFactory.sol (line 145)

TYPOS

     ///@audit: `priviliged` 
     * @dev     - priviliged internal function
  • File: contracts/modules/credit/SpigotedLine.sol (line 180)
     ///@audit: `priviliged` 
     * @dev     - priviliged internal function
  • File: contracts/modules/credit/SpigotedLine.sol (line 179)
Other instances of this issue are :

Use of block.timestamp

Block timestamps have historically been used for a variety of applications, such as entropy for random numbers, locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.

        uint256 timespan = block.timestamp - rate.lastAccrued;

File: contracts/modules/interest-rate/InterestRateCredit.sol (line 48)

        rates[id].lastAccrued = block.timestamp;

File: contracts/modules/interest-rate/InterestRateCredit.sol (line 50)

Other instances of this issue are :

Require /revert should have descriptive reason strings

    require(escrow_.liquidate(amount, targetToken, to));

File: contracts/modules/credit/EscrowedLine.sol (line 64)

    require(escrow.updateLine(newLine));

File: contracts/modules/credit/EscrowedLine.sol (line 90)

Other instances of this issue are :

Set garbage value in mapping for deleting that

If there is a mapping data structure present inside struct, then deleting the struct doesn't delete the mapping. Instead one should use lock to lock that data structure from further use.

        delete unusedTokens[token];

File: contracts/modules/credit/SpigotedLine.sol (line 257)

        delete mutualConsents[expectedHash];

File: contracts/utils/MutualConsent.sol (line 57)

Scientific notation

For readability, it is best to use scientific notation (e.g 10e5) rather than decimal literals(100000) or exponentiation(10**5)

        uint256 _numerator = collateralValue * 10**5; // scale to 4 decimals

File: contracts/utils/EscrowLib.sol (line 42)

NatSpec is incomplete

    /// @audit Missing: '@param'
    /**
     * @return price - the latest price in USD to 8 decimals
     */
    function getLatestAnswer(address token) external returns (int) {


    /// @audit Missing: '@param' & `return`
    /**
     *   @dev   We don't transfer the ownership of Escrow and Spigot internally
     *          because they aren't owned by the factory, the responsibility falls
     *          on the [owner of the line]
     *   @dev   The `cratio` in the CoreParams are not used, due to the fact
     *          they're passed in when the Escrow is created separately.
     */
    function deploySecuredLineWithModules(


Other instances of this issue are:

#0 - c4-judge

2022-12-07T16:29:58Z

dmvt marked the issue as grade-b

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0x1f8b, 0xRajkumar, Awesome, Aymen0909, B2, Bnke0x0, Deivitto, Diana, JC, Metatron, Rahoz, RaymondFam, RedOneN, ReyAdmirado, Rolezn, Saintcode_, TomJ, __141345__, ajtra, aphak5010, brgltd, c3phas, ch0bu, chrisdior4, cryptonue, durianSausage, emrekocak, erictee, exolorkistis, gogo, karanctf, lukris02, martin, me_na0mi, oyc_109, peanuts, rotcivegaf, saneryee, seyni, tnevler, zaskoh

Awards

49.2315 USDC - $49.23

Labels

bug
G (Gas Optimization)
grade-b
G-31

External Links

Link to GitHub issue

Multiple key mappings can be combined into a single mapping of key to a struct, where appropriate.

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot

    mapping(address => bool) enabled;
    /// tokens used as collateral (must be able to value with oracle)
    mapping(address => IEscrow.Deposit) deposited;

File: contracts/utils/EscrowLib.sol (line 16-18)

    mapping(address => uint256) escrowed; // token  -> amount escrowed
    mapping(address => ISpigot.Setting) settings; // revenue contract -> settings

contracts/utils/SpigotLib.sol

Use calldata instead of memory

If a reference type function parameter is read-only, it is cheaper in gas to use calldata instead of memory. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory.

Try to use calldata as a data location because it will avoid copies and also makes sure that the data cannot be modified.

    ILineOfCredit.Credit memory credit,

File: contracts/utils/CreditLib.sol (line 74)

    function addSpigot(address revenueContract, Setting memory setting) external returns (bool) {

File: contracts/modules/spigot/Spigot.sol (line 125)

Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

    function updateSplit(address spigot, address revenueContract, LineLib.STATUS status, uint8 defaultSplit) external returns (bool) {
        (uint8 split,, bytes4 transferFunc) = ISpigot(spigot).getSetting(revenueContract);

File: contracts/utils/SpigotedLineLib.sol (line 169-170)

	Other instances of the issue are:

internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

    function _mutualConsent(address _signerOne, address _signerTwo) internal returns(bool) {

File: contracts/utils/MutualConsent.sol (line 38)

	Other instances of the issue are:

Use unchecked to save gas

            unusedTokens[credit.token] -= repaid - newTokens;

File: contracts/modules/credit/SpigotedLine.sol (line 122)

            unusedTokens[credit.token] += newTokens - repaid;

File: contracts/modules/credit/SpigotedLine.sol#L125 (line 125)

	Other instances of the issue are:

abi.encode() is less efficient than abi.encodePacked()

    return keccak256(abi.encode(line, lender, token));

File: contracts/utils/CreditLib.sol (line 69)

UNUSED NAMED RETURNS

Using both named returns and a return statement isn’t necessary. Removing one of those can improve code clarity:

    returns (ILineOfCredit.Credit memory c, uint256 principal, uint256 interest)

File: contracts/utils/CreditLib.sol (line 80)

      returns(ILineOfCredit.Credit memory credit)

File: contracts/utils/CreditLib.sol (line 133)

#0 - c4-judge

2022-11-17T23:03:52Z

dmvt marked the issue as grade-b

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