hats-finance / Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf

Circles Protocol contracts
https://aboutcircles.com
GNU Affero General Public License v3.0
0 stars 0 forks source link

The last inserted avatar will not be updated during migration due to incorrect avatar insertion #45

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

Github username: @djanerch Twitter username: djanerch Submission hash (on-chain): 0x6179b76a8054492237dee10e3bdfa3bd18bec3276a3c05b6f8b968eb94868e0b Severity: medium

Description: Description \ When the _insertAvatar function is called, a new avatar is inserted into the avatars array. However, the function is implemented incorrectly. It should record the new avatars and associate them with their respective addresses. Instead, it incorrectly associates the last inserted address with the new record, rather than using the correct address.

Attack Scenario \ Let’s assume we are working with the address 0x10 and the last added address is 0x9. The function will create a record for avatars[0x10] but assign it the value of 0x9, causing the wrong address to be recorded. As a result, during migration, our address will not be updated correctly.

For instance, in the following part of the migrate function, where the user passes an array of avatars (which may come from the internal avatars array), the function iterates over the array and retrieves the recorded addresses. Due to the incorrect recording, the last address will not be updated:

for (uint256 i = 0; i < _avatars.length; i++) {
    // Mint the migrated balances to _owner
    _mintAndUpdateTotalSupply(_owner, toTokenId(_avatars[i]), _amounts[i], "");
}

This will result in an incorrect migration where the last recorded avatar address is not properly updated.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

Files:

benjaminbollen commented 1 week ago

Thank you for your report on the avatar insertion during migration. After careful review, we've determined that this is not an issue.

The behavior you've observed is due to the intentional design of our avatar storage system. Avatars are stored as a linked list to ensure iterability. This structure allows for efficient traversal and management of avatars within our system.

We appreciate your attention to detail in examining our migration process. Your thorough analysis of our data structures contributes to the overall understanding of our system's architecture. Thank you for your diligence in reviewing our implementation.