Summary

Incorrect handling of inactive elements

Vulnerability Details

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/starknet/src/bridge.cairo#L502C14-L513C19

The issue with the loop above is that it breaks as soon as it encounters an inactive element. This behavior assumes that all inactive elements should be at the end of the list, which may not always be the case.

The purpose of this loop is to find the last active element in the list to insert the new element after it. However, by breaking at the first inactive element, it inserts the new element in the middle of the list instead of at the end. Over time, this could lead to a fragmented list where active and inactive elements are interspersed, rather than having all active elements grouped together.

Suppose we have a list: A (active) -> B (inactive) -> C (active) -> D (active)

If we're trying to add a new active element E, the current code would break when it reaches B (inactive), and insert E after A. The resulting list would be:

A (active) -> E (active) -> B (inactive) -> C (active) -> D (active)

E would be expected to be added at the end of all active elements, like this:

A (active) -> C (active) -> D (active) -> E (active) -> B (inactive)

Impact

Active elements would appear after inactive ones in the list.

Tools Used

Manual review

Recommendations

Modify the loop to continue until it reaches the end of the list, regardless of whether elements are active or inactive. Then, insert the new element after the last active element.