Laksmana - Incorrect parameter filled in, causes ``LockstakeMkr`` to not minted

[sherlock-admin3]

Laksmana

Medium

Incorrect parameter filled in, causes LockstakeMkr to not minted

Summary

LockstakeMkr#mint is triggered from LockstakeEngine while those contracts only have the same wards. Thus, when LockstakeEngine calls LockstakeMkr#mint it will fail.

Root Cause

   ScriptTools.switchOwner(lockstakeInstance.lsmkr, deployer, owner);
   ScriptTools.switchOwner(lockstakeInstance.engine, deployer, owner);

Look at that code, the LockstakeMkr and LockstakeEngine when deployed they will have same wards. LockstakeMkr#mint using auth modifier:

    function mint(address to, uint256 value) external auth {

Meanwhile this function is triggred from LockstakeEngine.

As a result, when LockstakeEngine triggers LockstakeMkr#mint, it will fail.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

LockstakeMkr cannot be minted.

PoC

Paste this poc code into the test directory

run with forge test --match-test test_cant_minted_from_Engine

pragma solidity ^0.8.21;

import "dss-test/DssTest.sol";
import { LockstakeMkr } from "src/LockstakeMkr.sol";

contract Test_cant_minting is Test {
    LockstakeEngineMockForAuth lockstakeEngineMock;
    LockstakeMkr lsmkr;
    address owner;

    function setUp() public {
        owner = makeAddr("owner");
        lsmkr = new LockstakeMkr();
        lockstakeEngineMock = new LockstakeEngineMockForAuth(address(lsmkr));
        switchOwner(address(lockstakeEngineMock), address(this), owner);
        switchOwner(address(lsmkr), address(this), owner);
    }

    function test_cant_minted_from_Engine() public {
        vm.startPrank(owner);
        assertEq(lockstakeEngineMock.wards(owner), 1);
        assertEq(lsmkr.wards(owner), 1);

        vm.expectRevert("LockstakeMkr/not-authorized");
        lockstakeEngineMock.lock();

        vm.expectRevert("LockstakeMkr/not-authorized");
        lockstakeEngineMock.onRemove();

    }

    function switchOwner(
        address base,
        address deployer,
        address newOwner
    ) internal {
        if (deployer == newOwner) return;
        require(
            WardsAbstract(base).wards(deployer) == 1,
            "deployer-not-authed"
        );
        WardsAbstract(base).rely(newOwner);
        WardsAbstract(base).deny(deployer);
    }
}

interface WardsAbstract {
    function wards(address) external view returns (uint256);

    function rely(address) external;

    function deny(address) external;
}

contract LockstakeEngineMockForAuth {
    event Rely(address indexed usr);
    event Deny(address indexed usr);

    mapping(address usr => uint256 allowed) public wards;
    LockstakeMkr public lockstakeMkr;

    modifier auth() {
        require(wards[msg.sender] == 1, "LockstakeEngine/not-authorized");
        _;
    }

    constructor(address _lsmkr) {
       lockstakeMkr = LockstakeMkr(_lsmkr);
        wards[msg.sender] = 1;
    }

    function rely(address usr) external auth {
        wards[usr] = 1;
        emit Rely(usr);
    }

    function deny(address usr) external auth {
        wards[usr] = 0;
        emit Deny(usr);
    }

        function lock() external auth {
            lockstakeMkr.mint(msg.sender, 10e18);
        }

    function onRemove() external auth {
            lockstakeMkr.mint(msg.sender, 10e18);
    }
}

Mitigation

Fix like this:

        ScriptTools.switchOwner(lockstakeInstance.lsmkr, deployer, lockstakeInstance.engine);
        ScriptTools.switchOwner(lockstakeInstance.engine, deployer, owner);

[telome]

LockstakeInit.sol includes a line that sets the engine as a ward of LockstakeMkr.