Members for not yet existing community can be added #78
Labels
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
duplicate
This issue or pull request already exists
valid
Lines of code
https://github.com/code-423n4/2022-08-rigor/blob/e35f5f61be9ff4b8dc5153e313419ac42964d1fd/contracts/Community.sol#L184
Vulnerability details
Impact
When a
_communityID
that does not exist yet is passed toaddMember
,_community.owner
will beaddress(0)
.SignatureDecoder.recoverKey
(that is used withincheckSignatureValidity
) returnsaddress(0)
for invalid signatures. Therefore, it is easily possible to add a member to a community that does not exist yet (i.e. with a community ID higher than the current maximum value), as the signature validation succeeds for the owner when an invalid signature is passed. This has two negative consequences:1.) Members can add itself to a community without the permission of the future owner, which is of course not desired.
2.) As soon as the community is created, the entry in the
members
array is overwritten andmemberCount
is set to 1. However, theisMember
entry remainstrue
. Therefore, we havemembersCount = 1
,members.length = 1
, but there are two addresses such thatisMember[addr] = true
(the new owner and the previously added member). If multiple (say N) members are added previously, the consequences are even worse, as we will havemembersCount = 1
,members.length = N
and (N + 1) addresses whereisMember
is true. This breaks invariants of the protocol (that are not fixable by anyone). For instance, it enables situations where an address is not returned inmembers(communityID)
, but can still callpublishProject
, asisMember
is true for the address.Recommended Mitigation Steps
Only allow adding members for communities that exist.
The text was updated successfully, but these errors were encountered: