Wild Cinnamon Crocodile - Incorrect Values Emitted in NoncesIncremented Event

[sherlock-admin3]

Wild Cinnamon Crocodile

Low/Info

Incorrect Values Emitted in NoncesIncremented Event

Summary

• The incrementNonces function in the smart contract contains a bug where the values of lendingNonce and borrowingNonce emitted in the NoncesIncremented event are not the incremented values. This can cause confusion and inconsistency, as the emitted values will reflect the old nonces before the increment operation, while the actual state in the contract will store the incremented values.

Root Cause

Vulnerable Function: predict-dot-loan/contracts/PredictDotLoan.sol#L687

function incrementNonces(bool lending, bool borrowing) external {
    if (!lending && !borrowing) {
        revert NotIncrementing();
    }

    uint128 lendingNonce = nonces[msg.sender].lending;
    uint128 borrowingNonce = nonces[msg.sender].borrowing;

    if (lending) {
        unchecked {
            nonces[msg.sender].lending = ++lendingNonce;
        }
    }

    if (borrowing) {
        unchecked {
            nonces[msg.sender].borrowing = ++borrowingNonce;
        }
    }

    emit NoncesIncremented(lendingNonce, borrowingNonce);  // Bug: Emitting old values
}

Impact:

• Users or off-chain systems relying on the event to track updated nonce values will receive outdated information. This can lead to issues in tracking state changes, resulting in incorrect accounting or logic based on stale data.

PoC

1. Call the incrementNonces function with either lending or borrowing set to true.
2. Observe that the emitted values in the NoncesIncremented event correspond to the old values of the nonces (before increment), but the contract's storage reflects the updated values.

Mitigation

Proposed Fix:

Modify the emit statement to use the updated nonce values from storage rather than the local lendingNonce and borrowingNonce variables. The correct version is as follows:

function incrementNonces(bool lending, bool borrowing) external {
    if (!lending && !borrowing) {
        revert NotIncrementing();
    }

    uint128 lendingNonce = nonces[msg.sender].lending;
    uint128 borrowingNonce = nonces[msg.sender].borrowing;

    if (lending) {
        unchecked {
            nonces[msg.sender].lending = ++lendingNonce;
        }
    }

    if (borrowing) {
        unchecked {
            nonces[msg.sender].borrowing = ++borrowingNonce;
        }
    }

    // Emit the updated values from storage
    emit NoncesIncremented(nonces[msg.sender].lending, nonces[msg.sender].borrowing);
}