semaphore-protocol / semaphore

A zero-knowledge protocol for anonymous interactions.
https://semaphore.pse.dev
MIT License
887 stars 194 forks source link

unintended behavior for updating removed notes #720

Closed 0xDatapunk closed 5 months ago

0xDatapunk commented 5 months ago

Describe the bug Current _update(..) function does not check if oldLeaf != 0. https://github.com/privacy-scaling-explorations/zk-kit/blob/9001fdefaf1b9d71250ac1604b8e828c464ffc93/packages/imt.sol/contracts/internal/InternalLeanIMT.sol#L209

The result is if one node is removed, it can be updated to a new leaf, however, If more than one nodes were removed, only the last one can be updated to a valid leaf.

Discussed with @cedoor , updating removed node is not an expected behavior.

To Reproduce Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior check the old leaf with oldLeaf != 0, so that removed node cannot be updated.

Screenshots If applicable, add screenshots to help explain your problem.

Technologies (please complete the following information):

Additional context Add any other context about the problem here.