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
533 stars 179 forks source link

[Malleability] CollectionGuarantee #6722

Open UlyanaAndrukhiv opened 2 weeks ago

UlyanaAndrukhiv commented 2 weeks ago

CollectionGuarantee Malleability

https://github.com/onflow/flow-go/blob/edf27b03b6f809ce66cacd607a41b889c12cd2b3/model/flow/collectionGuarantee.go#L7-L15

The current CollectionGuarantee implementation uses the ID() method to return the CollectionID as the unique identifier: https://github.com/onflow/flow-go/blob/edf27b03b6f809ce66cacd607a41b889c12cd2b3/model/flow/collectionGuarantee.go#L18-L20

This method assumes that CollectionID is sufficient to uniquely identify a CollectionGuarantee. While this approach is simple, it introduces potential malleability concerns. The identifier computation currently only considers the CollectionID field, leaving out important fields like:

This omission means that two CollectionGuarantee instances with the same CollectionID but differing values for these fields would produce the same identifier, which is incorrect.

Proposed Solution

  1. Separate mutable and immutable fields: Create a CollectionGuaranteeBody struct containing the immutable fields:

    • CollectionID
    • ReferenceBlockID,
    • ChainID.
  2. Implement ID() for CollectionGuaranteeBody: Use the MakeID() function to compute the identifier for CollectionGuaranteeBody. All fields will be encoded consistently:

    • Primitive and byte array fields (CollectionID, ReferenceBlockID, ChainID) are encoded directly as raw bytes using RLP (encoding RLP rules).
  3. Update theCollectionGuarantee struct: Include the CollectionGuaranteeBody struct within CollectionGuarantee.

    type CollectionGuarantee struct {
    Body             CollectionGuaranteeBody
    SignerIndices    []byte           // encoded indices of the signers
    Signature        crypto.Signature // guarantor signatures
    }
  4. Update ID() for CollectionGuarantee: The ID() method will compute the identifier based on the entire CollectionGuarantee struct using MakeID().

    func (cg *CollectionGuarantee) ID() Identifier {
    body := struct {
        BodyID         flow.Identifier
        SignerIndices    []byte          
            Signature        crypto.Signature
    }{
        BodyID:         cg.Body.ID(),
        SignerIndices:   cg.SignerIndices,
        Signature:     cg.Signature,
    }
    return MakeID(body)
    }

    By encoding all fields, this approach guarantees that the resulting ID is malleability-resistant. Changes to any field of CollectionGuarantee will produce a different Identifier.

  5. Remove unused function: The unused Checksum() function will be removed.

Potential problems

Definition of Done

  1. The CollectionGuaranteeBody has been created and CollectionGuaranteeBody.ID() method has been implemented.
  2. The CollectionGuarantee.ID() method has been updated using MakeID.
  3. The unused Checksum() functions has been removed.
  4. Unit tests have been updated to validate the new behavior, ensuring identifiers change as expected when data is modified.
  5. Documentation and comments have been updated to reflect the changes and clarify the purpose of the ID() method.