sherlock-audit / 2024-05-aleo-judging

0 stars 0 forks source link

joicygiore - `credits.record::join` lacks `owner` validation, which may result in merging records with different owners #19

Closed sherlock-admin3 closed 1 week ago

sherlock-admin3 commented 2 weeks ago

joicygiore

Medium

credits.record::join lacks owner validation, which may result in merging records with different owners

Summary

credits.record::join lacks owner validation, which may result in merging records with different owners

Vulnerability Detail

credits.record::join lacks the check credits.record.owner, merging records with different owners may lead to data inconsistency or logic errors.

// The `join` function combines two records into one.
function join:
    // Input the first record.
    input r0 as credits.record;
    // Input the second record.
    input r1 as credits.record;
    // Combines the amount of the first record and the second record.
    // This `add` operation is safe, and the proof will fail
    // if an overflow occurs.
@>    add r0.microcredits r1.microcredits into r2;
    // Construct a record with the combined amount.
@>    cast r0.owner r2 into r3 as credits.record;
    // Output the record.
    output r3 as credits.record;

As a comparison, we can refer to the relevant methods in large_functions.aleo, which will check whether credits.record.owner is equal

function join_3:
    input r0 as credits.aleo/credits.record;
    input r1 as credits.aleo/credits.record;
    input r2 as credits.aleo/credits.record;
@>    assert.eq r0.owner r1.owner;
@>    assert.eq r1.owner r2.owner;
    call credits.aleo/join r0 r1 into r3;
    call credits.aleo/join r3 r2 into r4;
    output r4 as credits.aleo/credits.record;

Impact

credits.record::join lacks owner validation, which may result in merging records with different owners

Code Snippet

https://github.com/sherlock-audit/2024-05-aleo/blob/main/snarkVM/synthesizer/program/src/resources/credits.aleo#L930-L943 https://github.com/sherlock-audit/2024-05-aleo/blob/main/snarkVM/synthesizer/process/src/resources/large_functions.aleo#L5-L13

Tool used

Manual Review

Recommendation

Add owner check

function join:
    // Input the first record.
    input r0 as credits.record;
    // Input the second record.
    input r1 as credits.record;
    // Combines the amount of the first record and the second record.
    // This `add` operation is safe, and the proof will fail
    // if an overflow occurs.
+   assert.eq r0.owner r1.owner;
    add r0.microcredits r1.microcredits into r2;
    // Construct a record with the combined amount.
    cast r0.owner r2 into r3 as credits.record;
    // Output the record.
    output r3 as credits.record;
evanmarshall commented 1 week ago

This misunderstands how records work in Aleo. Records can only be spent by the owner by default as they are encrypted and signature verification happens as part of the proof generation.

It's not possible to spend a record where record.owner != self.signer