Incorrect handling of inner loop could lead to DoS

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-01-opus/blob/main/src/core/shrine.cairo#L2071

Vulnerability details

The issue arises from the lack of a clear exit condition in the inner loop that iterates over original_yang_balances_copy. The loop continues as long as original_yang_balances_copy.pop_front() returns Some(original_yang_balance). If this condition is met indefinitely, the loop will continue without end, leading to an infinite loop. This could potentially cause the smart contract to hang or consume excessive resources.

Vulnerable Code

https://github.com/code-423n4/2024-01-opus/blob/main/src/core/shrine.cairo#L2071

let mut original_yang_balances_copy = trove_yang_balances;
// Inner loop over all yangs
loop {
    match original_yang_balances_copy.pop_front() {
        Option::Some(original_yang_balance) => {
            // Code continues...
        }
        // No explicit exit condition shown
    }
}

Impact

• DoS: An infinite loop could consume excessive computational resources, potentially leading to a denial of service.

Mitigation

An explicit exit condition should be added to the inner loop. This could be achieved by checking the length of original_yang_balances_copy at the start of each iteration and breaking the loop if the length is zero. Alternatively, the loop could be refactored to a for-loop that iterates over the elements of original_yang_balances_copy directly, which would automatically terminate once all elements have been processed.

Assessed type

DoS

[tserg]

Invalid - see https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/shrine.cairo#L2185.

[c4-pre-sort]

bytes032 marked the issue as insufficient quality report

[alex-ppg]

The Warden denotes how a loop that pops elements from an array may result in a DoS due to an infinite loop; this is invalid as an array in Cairo by definition is finite. As the loop breaks whenever an element is not found, the code will properly break and continue its execution when necessary rendering this submission invalid.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid