Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AddPartyCardsAuthority has no function to abdicateAuthority #511

Open
c4-submissions opened this issue Nov 10, 2023 · 5 comments
Open

AddPartyCardsAuthority has no function to abdicateAuthority #511

c4-submissions opened this issue Nov 10, 2023 · 5 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-a insufficient quality report This report is not of sufficient quality QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@c4-submissions
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernanceNFT.sol#L486
https://github.com/code-423n4/2023-10-party/blob/main/contracts/authorities/AddPartyCardsAuthority.sol#L7

Vulnerability details

Impact

Parties have an abdicateAuthority function which relinquishes authority role for msg.sender.
The issue is AddPartyCardsAuthority does not have a mechanism to call the abdicateAuthority function, making the function useless as it is never called

Proof of Concept

Parties create proposals to add an Authority. These authorities have privileges like altering votingPower of a user or totalVotingPower of a party.

Parties have an abdicateAuthority function, which is expected to be called by an authority to give up the authority role:

    function abdicateAuthority() external {
        _assertAuthority();
        delete isAuthority[msg.sender];

        emit AuthorityRemoved(msg.sender);
    }

The problem is that, the current implementations of authorities(AddPartyCardsAuthority), which protocol created(and is expected to be used by most parties), does not have a way to call the abdicateAuthority function.

Tools Used

Manual Review

Recommended Mitigation Steps

  • AddPartyCardsAuthority and future authorities should have a function that can call Party's abdicateAuthority function. The function should only be callable by the Party.
  • An Authority abstract contract should be created, which should contain all essential functions that an authority should have. All authority contracts should inherit this abstract contract and implement the essential functions.

Assessed type

Context

@c4-submissions c4-submissions added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Nov 10, 2023
c4-submissions added a commit that referenced this issue Nov 10, 2023
@ydspa
Copy link

ydspa commented Nov 11, 2023

QA: L

@c4-pre-sort
Copy link

ydspa marked the issue as insufficient quality report

@c4-pre-sort c4-pre-sort added the insufficient quality report This report is not of sufficient quality label Nov 11, 2023
@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Nov 19, 2023
@c4-judge
Copy link
Contributor

gzeon-c4 changed the severity to QA (Quality Assurance)

@Emedudu
Copy link

Emedudu commented Nov 22, 2023

Parties have an abdicateAuthority function, which is meant to be called by an authority to give up the authority role.

The AddPartyCardsAuthority, which is the only protocol developed authority, and will be used by most Parties, lack a means to call Party#abdicateAuthority.
This means that AddPartyCardsAuthority will be stuck with a Party forever, even when the developers intend that a Party should be able to switch authorities

This clearly shows that AddPartyCardsAuthority does not work as intended, as Party#abdicateAuthority is never called.

Therefore, I believe this should be of MEDIUM severity

Please have a look, thanks.

@gzeon-c4
Copy link

gzeon-c4 commented Nov 23, 2023

AddPartyCardsAuthority is out-of-scope
meant to invalidate this but its tricky to validate a qa issue with the c4 ui

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-a insufficient quality report This report is not of sufficient quality QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Development

No branches or pull requests

7 participants