Skip to content

Conversation

2color
Copy link
Member

@2color 2color commented Jul 21, 2025

Description

  • Added logic to handle truncated multihashes during verification in networked storage
  • Enhanced hash verification to work with partial hash values
  • Added test to cover truncated hash scenario

Background

Originally reported in https://ipshipyard.slack.com/archives/C0238P3SU4X/p1753096031053949

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

@2color 2color requested a review from a team as a code owner July 21, 2025 15:12
@2color 2color force-pushed the fix-truncated-multihashes branch from 68e97de to 4786963 Compare July 21, 2025 15:13
Comment on lines +414 to +419
if (hash.digest.length > cid.multihash.size) {
hash = {
...hash,
digest: hash.digest.subarray(0, cid.multihash.size)
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this without sufficient checks is not a good idea from a security perspective. Go has https://github.com/ipfs/boxo/tree/main/verifcid to help stop people from ending up in unsafe situations, it's used in places where you'd be importing data into the system whether the data is locally imported or coming from the network.

For some history on this see ipfs/kubo#4371 and linked issues / PRs.

@achingbrain
Copy link
Member

achingbrain commented Jul 22, 2025

I've opened multiformats/js-multiformats#329 (and multiformats/js-multiformats#328 for discussion) which implements this in multiformats and allows specifying limits on a per-hash basis (defaulting to https://github.com/ipfs/boxo/blob/main/verifcid/cid.go#L17-L20).

Here we would then just need to do:

const res = hasher.digest(block, {
  truncate: cid.multihash.size
})

...and it would throw if the truncation is too long/short/not allowed.

@2color
Copy link
Member Author

2color commented Jul 23, 2025

Closing in favour of multiformats/js-multiformats#329

@2color 2color closed this Jul 23, 2025
@achingbrain
Copy link
Member

multiformats/js-multiformats#329 has shipped so this can be accomplished via this now:

const res = hasher.digest(block, {
  truncate: cid.multihash.size
})

@agmap
Copy link

agmap commented Sep 2, 2025

Just a question 🤔:
Does this PR now needs to reopen after multiformats has shipped?
@2color @achingbrain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants