Escher contest - CRYP70's results

Lines of code

https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/OpenEdition.sol#L92 https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/LPDA.sol#L85 https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/LPDA.sol#L86 https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/LPDA.sol#L105

Vulnerability details

Description

The refund() function for example in the LPDA contract allows buyers to receive a refund on the difference between their current price of a non-fungible token (NFT) and the price at which they purchased it. This uses the payable(msg.sender).transfer(owed) method. There exists a flaw in the contracts implementation which can cause problems in the contract's overall gas consumption, potentially leading to unexpected errors and a loss of funds for both buyers and sellers.

Impact

This issue was assigned a Medium in severity because it can have consequences for the contract's gas usage and the overall performance of the Ethereum network however, certain edge case conditions must be met for transactions to fail. When payable() is called, it sets the stipend for the transfer to the caller's remaining gas, which means the contract has no control over how much gas is used for the transfer. This can lead to the contract running out of gas and failing, or to the contract consuming more gas than necessary and potentially causing congestion on the network. Additionally, the original transfer() function uses a fixed stipend of 2300 gas units, which may not be sufficient for some contracts to process the transfer. This can limit the contracts ability to interact with other contracts that require more gas to complete the transaction, potentially hindering its functionality. There is also no check to see if the transaction was successful which may result in a loss of user funds if a transaction fails.

Proof of Concept

This was identified in the following contracts:

• https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/OpenEdition.sol#L92
• https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/LPDA.sol#L85
• https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/LPDA.sol#L86
• https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/LPDA.sol#L105

Tools Used

Manual Review

Recommended Mitigation Steps

It's recommended that the contracts use a low level function call when transferring Ether between contracts and EOAs. This can be implemented using the following example:

(bool success,) = address(msg.sender).call{value: owed}("")
require(success, "Failed to refund Ether!")

Sources:

• https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

Lines of code

https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/LPDA.sol#L58-L89

Vulnerability details

Description

The buy() function in the LPDA contract is designed to allow users to purchase non-fungible tokens (NFTs) from a fixed-price sale. However, there is a critical flaw in the functions implementation which can allow users to make purchases even after the sale has ended, potentially causing financial loss for both the buyers and the sellers. Note that this was enforced here in the OpenEdition contract.

Impact

This was awarded a High in severity because the LPDA contract allows users to wait until the end of the sale to purchase tokens at a significantly reduced price, potentially leading to financial losses for early buyers who paid full price. Additionally, the dropPerSecond struct member in sale determines how quickly the price of the token will decrease as the sale continues and a high value for this parameter will result in users being able to acquire tokens for next to nothing. This could potentially lead to a loss of funds for all token holders as the influx of new tokens could decrease the overall value of the token supply.

Proof of Concept

The proof of concept Solidity test outlines the impact mentioned above:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import {EscherTest} from "./utils/EscherTest.sol";
import {LPDAFactory, LPDA} from "src/minters/LPDAFactory.sol";

import "../src/Escher721.sol";

contract LPDABase is EscherTest {
    LPDAFactory public lpdaFactory;
    LPDA.Sale public lpdaSale;
    LPDA public sale;

    address eve = vm.addr(9);
    address alice = vm.addr(8);
    address bob = vm.addr(7);

    function setUp() public virtual override {
        super.setUp();
        lpdaFactory = new LPDAFactory();
        // set up a LPDA Sale
        lpdaSale = LPDA.Sale({
            currentId: uint48(0),
            finalId: uint48(10),
            edition: address(edition),
            startPrice: uint80(uint256(1 ether)),
            finalPrice: uint80(uint256(0.1 ether)),
            dropPerSecond: uint80(uint256(0.1 ether) / 1 days),
            startTime: uint96(block.timestamp),
            saleReceiver: payable(address(alice)),
            endTime: uint96(block.timestamp + 7 days)
        });

        vm.deal(address(eve), 100 ether);
    }

    function testBuyAfterEndSale() public {
        // Setup LPDA Sale contract
        sale = LPDA(lpdaFactory.createLPDASale(lpdaSale));
        Escher721 nft = Escher721(address(edition));
        nft.grantRole(nft.MINTER_ROLE(), address(sale));

        // Warp to after the end of the sale. 
        vm.warp(block.timestamp + uint256(lpdaSale.endTime) + 2 days);

        // Eve waits until the sale finishes and is able to buy an NFT at a cheaper price
        vm.startPrank(address(eve));
        sale.buy{value: 1 ether}(1);
        sale.refund();
        vm.stopPrank();

        uint256 nftBalanceAfter = nft.balanceOf(address(eve));
        assertEq(nftBalanceAfter, 1);
        
    }

}

This issue was found in the LPDA.buy() function.

Tools Used

Manual review

Recommended Mitigation Steps

Similarly to that of the OpenEdition contract, it's recommended that the endTime struct member of Sale in the LPDA contract is enforced when attempting to buy a token. This can be simily be done by adding a require statement in the buy() function - see the example below:

function buy(uint256 _amount) external payable {
    uint48 amount = uint48(_amount);
    Sale memory temp = sale;
    require(block.timestamp < sale.endTime, "TOO LATE");
------------------------- SNIP -------------------------