dtheo - Aleo's zero address checks should be in bonding functions and not `unbond_public`

[sherlock-admin3]

dtheo

Medium

Aleo's zero address checks should be in bonding functions and not unbond_public

Summary

credits.aleo has checks in unbond_public to make sure that the validators withdrawal address is not set to the default dummy address. It even makes an extra variable (r8) to account for the corner case of the zero address actually calling the function. These checks would serve more purpose in bond_public and bond_validator as they could actually prevent loss of funds in these locations and would still cover this corner case.

Vulnerability Detail

If Aleo is going to add logic to handle the corner case of this address use (in this case pubkey aleo1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3ljyzc) then it should not place them in unbond_public. Aleo should move this check into bond_validator to prevent the address from ever being set to a validator withdrawal address since its address can never be known. This will prevent the loss of funds (100 credit minimum validator bond) that would happen in this case and would reduce the complexity of unbond_public, which is called by both validators and delegators alike.

Impact

The check where it currently stands in unbond_public does not prevent a loss of funds from the validator but is still required to preserve correct logic of unbonding delegator bonds. If a validator does somehow set this address as their withdrawal address the mimimum loss of funds is 100 credits.

Code Snippet

snarkVM/synthesizer/program/src/resources/credits.aleo#L460-L465

I have also created an annotated pseudo code version of unbond_public to make it easier to read.

Tool used

Manual Review

Recommendation

Add a check that the withdrawal addresss being set in bond_validator and bond_public are not equal to aleo1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3ljyzc. Refactor unbond_public to remove the handling of this corner case.

[evanmarshall]

I'd categorize this as informational or minor. We should never be in a situation where the aleo1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3ljyzc address is set as the withdrawal key, if anything we could probably remove the edge case handling from unbond_public

[infosecual]

Just noting here that this is likely worth fixing but was not escalated because sherlock judging rules explicitly call out zero address checks as ineligible.