It is possible for `shortName` to be calculated as the 0 bytes, breaking certain invariants

[hats-bug-reporter[bot]]

Github username: @yixxas Twitter username: yixxas Submission hash (on-chain): 0xb503fb513a63e8fc3139d03c71a2528c8f1e0ab859d2083c7e0598074474b3b1 Severity: low

Description: Description\ A shortName is psuedo randomly created by calculating the keccak256 hash of the concatenation of _avatar and nonce. This value is restricted in the space of ~72 bytes (closer to 70 bytes due to MAX_SHORT_NAME). There is no check for which the calculated value is not 0.

Considering that the search space of 72 bytes is relatively small compared to the typical 256 bytes used, the probability of this happening is not statistically insignifcant.

Attack Scenario\ When a calculated short name happens to be 0, it will break some invariants of the protocol, creating confusions for users.

1. The check for whether a short name already exist is checked by ensuring that the value in shortNames[] is not 0. Hence, a user who created a short name with the 0 value will be considered to have not created one, despite the call to registerShortName() being successful. They can create a new shortName by making another call. This particular user will have the event RegisterShortName() created twice on the same avatar address, possibly creating confusions for offchain bots seaching for events.
2. The value in shortNameToAvatar[0] will be set to the first avatar's address whose value is created where shortName == 0. This value will never be changed as future creation of shortName == 0 will no longer be possible. The invariant where shortNameToAvatar[0] = 0 is broken.

Recommendation A simple fix can be implemented by adding a check to ensure that calculated shortName is not 0 in both _registerShortName() and _registerShortNameWithNonce().

[benjaminbollen]

1. I disagree with it being an attack, because if someone wants to brute-force the nonce to find that zero-short name, ... go for it. There is really no damage.
2. For a moment I wanted to consider it as a "low" issue because of your argument 2 that it breaks the (irrelevant) invariant (but an invariant nonetheless) which I think is a valid point
3. but then I ran some numbers.
   • If this would have been a sha256, and one had a modern ASIC 110TH/s ($2000 cost) (my numbers might be all wrong here, but i googled and did some quick back of the envelope estimates);
   • You'd roughly have to run it for 1.3 years and with $0.10 kWh an electricity cost of say ~3k USD; so for some 5k USD, yeah sure someone could brute force it for lolz.
   • But it's not sha256, it's keccak256, and suddenly you have a barren waste land of options.
   • The best numbers I could find is ~1.4GH/s on a proper GPU (again, please correct me if Im wrong on my numbers);
   • and you're at 107.000 years with one GPU, or some 100.000 GPUs to bring it down to one year; and ~27m USD to run the electricity bill for that year (and some 60m USD to buy a GPU farm)

I appreciate highlighting the possible code check, but I can't acknowledge this as an issue

[benjaminbollen]

On 4th consideration, I do want to thank and reward a solid effort to find a solid code improvement. So on the argument that it breaks an invariant (in the reverse map) I'll agree with you on a low issue.