onflow / flow-go

A fast, secure, and developer-friendly blockchain built to support the next generation of games, apps, and the digital assets that power them.
GNU Affero General Public License v3.0
534 stars 179 forks source link

[Malleability] Collection #6721

Open UlyanaAndrukhiv opened 1 week ago

UlyanaAndrukhiv commented 1 week ago

Collection Malleability

https://github.com/onflow/flow-go/blob/edf27b03b6f809ce66cacd607a41b889c12cd2b3/model/flow/collection.go#L5-L8

The current Collection implementation uses the ID() method to compute the identifier for a collection by delegating the call to the ID() method of the corresponding LightCollection. This approach introduces potential malleability concerns. Specifically, it relies on LightCollection, which only references the TransactionIDs and excludes important details about the transactions themselves. This could result in identifiers that do not fully represent the entire Collection and its underlying data. https://github.com/onflow/flow-go/blob/edf27b03b6f809ce66cacd607a41b889c12cd2b3/model/flow/collection.go#L36-L38

Proposed Solution

  1. Update ID(): To address these concerns, the ID() method will be updated to compute the identifier based on the entire Collection instead of relying on c.Light().ID(). This ensures that the identifier includes all the information encapsulated within the Collection, not just the light references to transaction IDs.

It is safe to use MakeID() for this purpose because the Fingerprint() method is already implemented for Collection. This guarantees that MakeID will generate a unique and malleability-resistant identifier by considering all relevant fields within the Collection.

func (c *Collection) ID() Identifier {
    return MakeID(c)
}
  1. Remove unused function: Checksum() function will be removed.

Definition of Done

  1. The Collection.ID() method has been updated using MakeID.
  2. All references to Light().ID() in the context of Collection.ID() have been removed, the unused Checksum() function has been removed.
  3. Unit tests and usages have been updated to verify that ID() considers the entire Collection for identifier computation.
  4. Documentation and comments have been updated to reflect the change, explaining the importance of this update in preventing malleability.