code-423n4 / 2022-01-trader-joe-findings

2 stars 0 forks source link

Separate issuer functions from regular users #294

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

pauliax

Vulnerability details

Impact

I think it would make sense to separate issuer functions from regular users, as usually it contains different logics anyway, e.g. function withdrawLiquidity():

    if (msg.sender == issuer) {
        balance = lpSupply / 2;

        emit IssuerLiquidityWithdrawn(msg.sender, address(pair), balance);

        if (tokenReserve > 0) {
            uint256 amount = tokenReserve;
            tokenReserve = 0;
            token.transfer(msg.sender, amount);
        }
    } else {
        emit UserLiquidityWithdrawn(msg.sender, address(pair), balance);
    }

The check that the msg.sender is issuer will be irrelevant >99% of the time (if we assume there will be many users), but it will consume extra gas every time. Also, it would help to keep a single responsibility principle. But the deployment costs might be a bit higher.