sherlock-audit / 2024-05-aleo-judging

0 stars 0 forks source link

haxatron - A validator cannot unbond any of its delegators if its not in the committee #9

Closed sherlock-admin2 closed 1 week ago

sherlock-admin2 commented 2 weeks ago

haxatron

Medium

A validator cannot unbond any of its delegators if its not in the committee

Summary

A validator cannot unbond any of its delegators if its not in the committee

Vulnerability Detail

A core functionality is that a validator can unbond a delegator even if the validator is not in the committee, as supported by the following comment:

https://github.com/sherlock-audit/2024-05-aleo/blob/main/snarkVM/synthesizer/program/src/resources/credits.aleo#L456-L458

    // Note: This contains check must use `withdraw` and **not** `committee` to ensure the validator
    // can unbond delegators even when the validator is not in the committee. Of course, if the validator is
    // in the committee, then the validator must be able to unbond its delegators.

But to unbond a delegator, unbond_public must be called from the validator withdrawal address.

https://github.com/sherlock-audit/2024-05-aleo/blob/main/snarkVM/synthesizer/program/src/resources/credits.aleo#L407-L515

// This function allows any staker to unbond their microcredits from a validator,
// callable only by using the withdrawal address of the staker or the validator.
//
// **Validators**: It will remove the validator if the self bonded balance falls below 100 credits
// or the total delegated balance falls below 10M credits.
// **Delegators**: It will remove the validator if the total delegated balance falls below 10M credits.
// It will remove the entire bond_state of the delegator if it falls below 10,000 credits.
//
// Validators are permitted to fully unbond any of their delegators. When a validator unbonds a delegator,
// the entire bonded balance is unbonded, regardless of the amount of microcredits and may end up removing the validator
// from the committee if the total delegated balance falls below 10M credits.
//
// The corresponding function for 'unbond_public' is 'claim_unbond_public'.
function unbond_public:
    // Input the staker's address.
    input r0 as address.public;
    // Input the amount of microcredits to unbond.
    input r1 as u64.public;

    // Unbond the specified amount of microcredits for the specified validator.
    async unbond_public self.caller r0 r1 into r2;
    // Output the finalize future.
    output r2 as credits.aleo/unbond_public.future;

finalize unbond_public:
    // Input the caller's address.
    input r0 as address.public;
    // Input the staker's address.
    input r1 as address.public;
    // Input the amount of microcredits to unbond.
    input r2 as u64.public;

    // Set the default unbonding state.
    add block.height 360u32 into r3;
    // Construct the default unbonding state.
    cast 0u64 r3 into r4 as unbond_state;

    // Retrieve the staker's bond state.
    // Note: If the bonded state does not exist, reject the transition.
    get bonded[r1] into r5;

    /* Check Caller's Permission */

    // Get the staker's withdrawal address.
    get withdraw[r1] into r6;
    // Check if the caller's address equals the staker's withdrawal address.
    is.eq r0 r6 into r7;

    // Check if the validator's withdrawal address has been set.
    // Note: This contains check must use `withdraw` and **not** `committee` to ensure the validator
    // can unbond delegators even when the validator is not in the committee. Of course, if the validator is
    // in the committee, then the validator must be able to unbond its delegators.
    contains withdraw[r5.validator] into r8;
    // Get the validator's withdrawal address from the bond state, using the zero address as the default.
    get.or_use withdraw[r5.validator] aleo1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3ljyzc into r9;
    // Check if the validator's withdrawal address matches the caller.
    is.eq r0 r9 into r10;
    // AND the withdraw address has been set (to prevent an edge case where a validator uses the zero address as a withdrawal address).
    and r8 r10 into r11;

    // Either the caller is the staker's withdrawal address OR the validator's withdrawal address is set to the caller.
    or r7 r11 into r12;
    // Enforce the permission as `true`.
    assert.eq r12 true;

    // Check if bonded to self (validator) or to a different address (delegator).
    is.eq r5.validator r1 into r13;
    branch.eq r13 true to unbond_validator;

    /* Unbond the Delegator */
    // {
        // Retrieve or initialize the unbonding state.
        get.or_use unbonding[r1] r4 into r14;
        // Retrieve the delegated amount in microcredits for the validator.
        get delegated[r5.validator] into r15;

        // Calculate new bonded microcredits.
        // Note: If the subtraction underflows, reject the transition.
        sub r5.microcredits r2 into r16;

        // Check if the delegator will fall below 10,000 bonded credits.
        lt r16 10_000_000_000u64 into r17;

        // If the validator is forcing the delegator to unbond OR the delegator will fall below 10,000 bonded credits.
        or r11 r17 into r18;

        // Determine the amount to unbond: requested amount if >= 10,000 credits, otherwise the full bonded amount.
        ternary r18 r5.microcredits r2 into r19;
        ...

When the function is called from the validator withdrawal address, r11 will be true which will make r18 true, the code will then proceed to unbond the delegator.

However, the validator withdrawal address can only be set in bond_validator, and in order to do this the validator must be in the committee which requires 10M total delegated credits.

https://github.com/sherlock-audit/2024-05-aleo/blob/main/snarkVM/synthesizer/program/src/resources/credits.aleo#L245-L269

    // if (new_validator)
    contains committee[r0] into r16;
    branch.eq r16 true to validator_in_committee;
    // {
        // Set the withdrawal address.
        // Note: This operation is only necessary on the first time for a validator entry, in order to initialize the value.
        set r4 into withdraw[r0];

        // Check if the initial bond amount is at least 100 credits.
        gte r2 100_000_000u64 into r17;
        // Ensure that the initial bond is at least 100 credits.
        assert.eq r17 true;

        // Get the committee size.
        get.or_use metadata[aleo1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3ljyzc] 0u32 into r18;
        // Increment the committee size by one.
        add r18 1u32 into r19;
        // Set the new committee size.
        set r19 into metadata[aleo1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3ljyzc];

        // Check if the validator exists in the unbonding state.
        contains unbonding[r0] into r20;
        // Ensure the validator currently is not unbonding.
        assert.eq r20 false;
    // }

As such when the validator is not in the committee, it cannot unbond any of its delegators because the validator withdrawal address is not set which will make r11 false.

Impact

Broken core contract functionality

Code Snippet

https://github.com/sherlock-audit/2024-05-aleo/blob/main/snarkVM/synthesizer/program/src/resources/credits.aleo#L245-L269

Tool used

Manual Review

Recommendation

Allow the validator to unbond their delegators from their own address if the withdrawal address is not set (which implies they are not in the committee.)

Duplicate of #20