Skip to content

Instantly share code, notes, and snippets.

@RideSolo
Created January 24, 2020 17:54
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save RideSolo/4eaceb82481bef06f1b48a2a6d2cf3e0 to your computer and use it in GitHub Desktop.
Save RideSolo/4eaceb82481bef06f1b48a2a6d2cf3e0 to your computer and use it in GitHub Desktop.

TheWALL v2 Audit Report.

1. Summary

This document is a security audit report performed by RideSolo, where TheWALL has been reviewed.

2. In scope

3. Findings

3 issues were reported including:

  • 2 medium severity issues.
  • 1 low severity issue.

3.1 Cluster Size

severity: medium

Description

The issue described here is still applicable, if a user wants to remove an area from the cluster that is located in the end of the cluster array, then it will be impossible since the array size might be too large.

The loop does not require the user to go through the full array, but like this the user will be forced to remove areas in the beginning of the cluster (even if he wants to keep them) just to remove an area in the latest positions, then add them back later on.

Code snippet

https://github.com/ridesoloAudit/TheWall/blob/8bd03c94dfc8ad9e1429b48da6cf3b51a7e59f19/thewallcore.sol#L422

3.2 Premium Computing

severity: medium

Description

The frequency at wich the commitSecret and updateSecret is called is important to the users. If a user that created new area want to rent or sell it and the owner didn't call commitSecret for his hash then isPremium will return false, resulting in a loss of 30% (fee charged) even if his nonce and secret will give him a premium area. If the secret is commited later, a possible new owner will have access to the premium privilege.

Recommendation

  • Lock the sell and rent functionalities and unlock them after the secret commit.
  • Automate the process using an oracle callback such as oracalize or chainlink.

3.3 Zero Address Check

severity: low

Description

  • Check coupons contract address to be different than zero inside the constructor of TheWallUsers.
  • Check the updated coupons contract address to be different than zero here.

Conclusion

All highlighted issues must be fixed.

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