This document is a security audit report performed by RideSolo, where WednesdayClub has been reviewed.
- SafeMath.sol github commit hash 92f2aab40a6e36bc85853f2a109e737e0868a3b8.
- WednesdayClub.sol github commit hash ab55e6f6fba0d5ce1c0b7274efa2e719e13fb1f3.
- WednesdayClubComment.sol github commit hash ee647b448090707cb33187f4fa282a75345da087.
- WednesdayClubPost.sol github commit hash 825e162d32dfdc50b2a63345be1f150ddf52f4f4.
- WednesdayClubUser.sol github commit hash 825e162d32dfdc50b2a63345be1f150ddf52f4f4.
- destructible.sol github commit hash 0c98cbc694e12ef044bedc1aa56c6a4f221903b7.
- ownable.sol github commit hash 0c98cbc694e12ef044bedc1aa56c6a4f221903b7.
- pausable.sol github commit hash 14ca8fa8d6ea69d06f4801194addc680d214a13a.
- repayable.sol github commit hash 92f2aab40a6e36bc85853f2a109e737e0868a3b8.
- tokenInterfaces.sol github commit hash 0c98cbc694e12ef044bedc1aa56c6a4f221903b7.
7 issues was reported:
- 2 High severity issues.
- 2 Low severity issues.
- 2 Owner Privileges.
- 1 note.
All the following functions have a high risk to throw for Out of Gas or Block Gas Limit since the post or comment Id to be deleted is saved inside an array and a loop is used to find it.
deleteUserPostdeleteIdFromPostIdsdeletePostdeleteUserCommentdeleteCommentdeleteAllPosts
The more a user uses the Dapp to share post or comment the longer the array will get, meaning that for the long term no work arround can be done by the owner to avoid it.
Same issue is applicable for unfollow function where the array is a list of following or followers and unblockUser where the array is a list of blocked users.
Please note that once the id is found no break is used to exit the loop.
Following up with the same error:
nukeMenukePostsnukeComments
Have the same issue where there might be not enough of gas to delete the whole list if it is long enough, also it might reach Block Gas Limit.
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L100
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L115
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L140
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L201
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L217
https://github.com/RideSolo/WednesdayClub/blob/master/src/WednesdayClub.sol#L268
https://github.com/RideSolo/WednesdayClub/blob/master/src/WednesdayClub.sol#L299
https://github.com/RideSolo/WednesdayClub/blob/master/src/WednesdayClub.sol#L324
https://github.com/RideSolo/WednesdayClub/blob/master/src/WednesdayClub.sol#L330
https://github.com/RideSolo/WednesdayClub/blob/master/src/WednesdayClub.sol#L339
Mappings can be used to keep track of the array ellements id without looking through the whole array for the a specific post or comment to delete.
Developers should be aware that when delete is applied on an array id the value assigned to that id is reset to zero but the array length stays the same, meaning that the dev should implement a logic where the last element on the array will replace the deleted id and the array length should be decremented by one.
A function such deleteAllPosts is useless and does not meet the goal set by the dev (following the comments deleteAllPosts in groups i.e. delete 100, then 100 again, etc - for saving on gas and incase to many) since the first 100 deleted post will be just set to zero but the array lenght of postIds will stay the same. If the total number of posts is high enough the transaction can reach block gas limit.
Please note that delete is also used in other functions, and the delete logic should be updated to a better one.
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L130
-
When creating a post using
writePostthe structure saved does not include_contentand_mediaparameters (except the emitted event nothing can retrace the post to its included media or content), developers should always keep the link to the IPFS data inside the post structure and make it immutable. -
Same issue is applicable for
writeComment, nothing directly link the content and the media to the comment id (except the emmited event).
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L50#L67
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L150#L168
- The inputted post id in
writePostis not the same final id that is used to track the post itself. - The same note is applicable for
writeComment.
This can lead users to confusion except if handled by the Dapp.
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L53
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L153
The fallback function is used to deposit ether to the contract, however it can be missused by users. Developers should create a dedicated function for deposit with a unique name.
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L44
- Pause/Unpause the posts, comments, likes and follow for users through
whenNotPaused. - Delete posts and comments.
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L100
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L115
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L140
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L201
https://github.com/aelchim/WednesdayClub/blob/master/src/WednesdayClub.sol#L217
https://github.com/RideSolo/WednesdayClub/blob/master/src/WednesdayClub.sol#L268
The audit contracts are not safe for deployment.