Laksmana - missing auth modifier, causing loss of user funds when lock at different ``urn``

[sherlock-admin2]

Laksmana

Medium

missing auth modifier, causing loss of user funds when lock at different urn

Summary

LockstakeEngine#lock and LockstakeEngine#lockNgt have no urnAuth(urn) modifier.

As a result, everyone can "lock" with someone's urn.

Unfortunately, once they "lock". They cannot claim they token, which is perform "free".

Root Cause

look at these function lock:

  function lock(address urn, uint256 wad, uint16 ref) external {
        mkr.transferFrom(msg.sender, address(this), wad);
        _lock(urn, wad, ref);
        emit Lock(urn, wad, ref);
    }

    function lockNgt(address urn, uint256 ngtWad, uint16 ref) external {
        ngt.transferFrom(msg.sender, address(this), ngtWad);
        mkrNgt.ngtToMkr(address(this), ngtWad);
        _lock(urn, ngtWad / mkrNgtRate, ref);
        emit LockNgt(urn, ngtWad, ref);
    }

So, everyone can perform "locking" with one's urn, since those functions do not use the urnAuth(urn) modifier.

The urnAuth(urn) modifier is for check that caller of urn owner or allowed to used urn.

Unfortunately, once they are "locked in", they cannot claim their token. Because the "free" function has urnAuth(urn). #### As a result, their token will belong to someone's urn that they used.

    function free(address urn, address to, uint256 wad) external urnAuth(urn) returns (uint256 freed) {

    function freeNgt(address urn, address to, uint256 ngtWad) external urnAuth(urn) returns (uint256 ngtFreed) {

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

As a result, users who lock with someone's urn, their tokens will belong to that person.

PoC

• paste this code to file LockstakeEngine.t.sol

• run with forge test --match-test test_lock_in_wrong_urn

  function test_lock_in_wrong_urn() public {
      address andi = makeAddr("andi");
      vm.prank(andi); 
      address urnAndi = engine.open(0);
  
      address jhon = makeAddr("jhon");
      vm.startPrank(jhon);
      deal(address(mkr), jhon, 100_000 * 10**18);
      mkr.approve(address(engine), 100_000 * 10**18);
      engine.lock(urnAndi, 100_000 * 10**18, 5);
  
      vm.expectRevert("LockstakeEngine/urn-not-authorized");
      engine.free(urnAndi, jhon, 100_000 * 10**18);
  
  }

Mitigation

+     function lock(address urn, uint256 wad, uint16 ref) external urnAuth(urn) {
+     function lockNgt(address urn, uint256 ngtWad, uint16 ref) external urnAuth(urn) {

[telome]

This is by design. From the competition rules: "Any user mistakes resulting in their own funds being lost is out of scope ".

[sherlock-admin3]

Escalate

This is by design. From the competition rules: "Any user mistakes resulting in their own funds being lost is out of scope ".

Before I submitted this report, I had read the competition rules.

So, why did I classify this issue as a medium issue and submit it?

• Not using urnAuth(urn) on lock and lockNgt functions is very suspicious.
• When users want to do “free” then urnAuth(urn) access is required.
• Let's say to make everyone can do “lock” without creating urn, but the owner of urn can give access to others with hope function. The urnAuth(urn) function will verify it. Then why doesn't makerdao use urnAuth(urn) in the lock function?

This is very suspicious for users. We assume that this is deliberately done so that a malicious internal actor can take the user's funds by changing the destination urn in the web frontend to belong to the internal actor when the user wants to lock.

When the victim protests, the malicious internal actor will counterattack the victim by saying that it is the user's fault when doing lock by entering the wrong urn, then proving the lock code as the reason. As a result, the malicious internal actor gets the user's funds. It may happen.

Makerdao is decentralized, so make it decentralized. Ensure user security by using urnAuth(urn) in the “lock” function

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page, in the Sherlock webapp.