mapping deletion will not remove the full content

[code423n4]

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/distribution/TokenDistributor.sol#L309

Vulnerability details

mapping deletion will not remove the full content

Summary

A deletion in a structure containing a mapping will not delete the mapping (see the Solidity documentation).

The remaining data may be used to compromise the contract.

Proof of Concept

Navigate to the following contract: https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/distribution/TokenDistributor.sol#L309

Recommended Mitigation Steps

• Use a lock mechanism instead of a deletion to disable structure containing a mapping.

[merklejerk]

We're going to delete this function entirely.

[trust1995]

I don't think this is a valid find. The relevant docs: delete has no effect on mappings (as the keys of mappings may be arbitrary and are generally unknown). So if you delete a struct, it will reset all members that are not mappings and also recurse into the members unless they are mappings. However, individual keys and what they map to can be deleted: If a is a mapping, then delete a[x] will delete the value stored at x. Since in this case contract is deleting a[x][y], the deletion is fine.

[merklejerk]

Since the struct does have a nested mapping, technically some state still persists :shrug:

[trust1995]

Valid point, there is still some state that survives. I do think that since warden did not specify anything of meaning to do with that leak, that it is be downgraded to low finding.

[0xble]

Resolved: https://github.com/PartyDAO/partybidV2/pull/125

[HardlyDifficult]

The storage here is keyed by party+id - so I don't see the relevance. Closing as invalid.