Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

verifyProof does not throw an error on invalid proofs #1368

Closed
brickpop opened this issue Jul 20, 2021 · 0 comments · Fixed by #1373
Closed

verifyProof does not throw an error on invalid proofs #1368

brickpop opened this issue Jul 20, 2021 · 0 comments · Fixed by #1373

Comments

@brickpop
Copy link

brickpop commented Jul 20, 2021

We are using the BaseTrie class to check the validity of Ethereum storage proofs and proof of non-existence.

According to the Readme, verifyProof should be throwing an error if proof is found to be invalid.

However:

  • We are setting invalid values to the proof array but no error is thrown
  • It is thrown is the root is the invalid value (expected)

By always receiving null as a response, we have no way to tell if it is because (1) the proof is valid and there is no such value or because (2) the proof is invalid and the value doesn't match anyway.

Steps to reproduce:

  • Adding the following test case to trie/test/index.spec.ts
tape('shall throw an error when a proof is invalid', async (t) => {
  const trie = new BaseTrie()
  await trie.put(Buffer.from('a'), Buffer.from('value1'))
  await trie.put(Buffer.from('aa'), Buffer.from('value2'))
  await trie.put(Buffer.from('aaa'), Buffer.from('value3'))

  const proof = await BaseTrie.createProof(trie, Buffer.from("a"))

  // Normal verification
  await BaseTrie.verifyProof(trie.root, Buffer.from('a'), proof)

  // Corrupting the proof
  proof[0].reverse()

  try {
    // @throws — If proof is found to be invalid.
    await BaseTrie.verifyProof(trie.root, Buffer.from('a'), proof)
    t.fail('should have thrown an error, but didn\'t')
  } catch (err) { }

  t.end()
})

Some testing for invalid roots should also be added.
Thank you!

(thanks @ed255 for the discovery)

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

Successfully merging a pull request may close this issue.

2 participants