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] ChunkDataPack #6720

Open UlyanaAndrukhiv opened 1 week ago

UlyanaAndrukhiv commented 1 week ago

ChunkDataPack Malleability

https://github.com/onflow/flow-go/blob/22daf5cdccbe3dc4af7e239495d54ed4c9d304f0/model/flow/chunk.go#L94-L104

The current ChunkDataPack implementation uses the ID() method to return the ChunkID field as the unique identifier: https://github.com/onflow/flow-go/blob/22daf5cdccbe3dc4af7e239495d54ed4c9d304f0/model/flow/chunk.go#L123-L127

This approach assumes that ChunkID is sufficient to uniquely identify a ChunkDataPack. While this method is straightforward, it introduces potential malleability concerns because it does not consider the integrity of the other fields within ChunkDataPack. Specifically, fields like:

are excluded from the identifier computation. This omission means that two ChunkDataPack instances with the same ChunkID but differing values for these fields would produce the same ID, which is incorrect.

Proposed Solution

Key Changes

  1. Update ID() : To address these concerns, the ID() method will be updated to compute the identifier based on the entire ChunkDataPack struct using the MakeID() function:
func (c *ChunkDataPack) ID() Identifier {
 body := struct {
         ChunkID    Identifier    
             StartState StateCommitment 
             Proof      StorageProof    
             Collection Identifier
             ExecutionDataRoot BlockExecutionDataRoot 
    }{

        ChunkID: c.ChunkID,
        StartState: c.StartState,
        Proof: c.Proof,
        Collection: c.Collection.ID(),
                ExecutionDataRoot: c.ExecutionDataRoot,

    }
    return MakeID(body)

}

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

All fields will be encoded in a way that guarantees consistency:

  1. Remove unused function: During the revision process, it was identified that the function FromChunkID is unused and redundant. This function will be removed. Also Checksum() function will be removed.

Definition of Done

  1. The ChunkDataPack.ID() method has been updated using MakeID. The identifier computation has been verified to use RLP encoding or Fingerprint for all fields.
  2. The unused FromChunkID and Checksum() functions has been removed.
  3. Unit tests have been updated to validate the new behavior, ensuring identifiers change as expected when data is modified.
  4. Documentation and comments have been updated to reflect the changes and clarify the purpose of the ID() method.