search

sherlock-audit / 2024-05-aleo-judging

0 stars 0 forks source link

haxatron - Using `self.signer` in `transfer_public_as_signer` can allow external malicious programs to drain Aleo credits #4

Closed sherlock-admin3 closed 1 week ago

sherlock-admin3 commented 2 weeks ago

haxatron

High

Using self.signer in transfer_public_as_signer can allow external malicious programs to drain Aleo credits

Summary

Using self.signer in transfer_public_as_signer can allow external malicious programs to drain Aleo credits

Vulnerability Detail

self.signer is equivalent to tx.origin, where the self.signer is the originator of the transaction.

This creates a huge risk when used in transfer_public_as_signer because a malicious Aleo program in the call flow can make an external call to transfer_public_as_signer in credits.aleo which will transfer the Aleo credits from the self.signer, the originator of the transaction to any receiver.

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

// The `transfer_public_as_signer` function sends the specified amount
// from the signer's `account` to the receiver's `account`.
function transfer_public_as_signer:
    // Input the receiver.
    input r0 as address.public;
    // Input the amount.
    input r1 as u64.public;
    // Transfer the credits publicly.
    async transfer_public_as_signer self.signer r0 r1 into r2;
    // Output the finalize future.
    output r2 as credits.aleo/transfer_public_as_signer.future;

finalize transfer_public_as_signer:
    // Input the signer.
    input r0 as address.public;
    // Input the receiver.
    input r1 as address.public;
    // Input the amount.
    input r2 as u64.public;
    // Decrements `account[r0]` by `r2`.
    // If `account[r0] - r2` underflows, `transfer_public_as_signer` is reverted.
    get account[r0] into r3;
    sub r3 r2 into r4;
    set r4 into account[r0];
    // Increments `account[r1]` by `r2`.
    // If `account[r1]` does not exist, 0u64 is used.
    // If `account[r1] + r2` overflows, `transfer_public_as_signer` is reverted.
    get.or_use account[r1] 0u64 into r5;
    add r5 r2 into r6;
    set r6 into account[r1];

Impact

As such, Aleo credits can be drained by malicious programs in the call flow this way.

Code Snippet

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

Tool used

Manual Review

Recommendation

Remove this method as it is dangerous.

Duplicate of #15