Skip to content

Instantly share code, notes, and snippets.

@greywolf12

greywolf12/Enduracoin_reaudit_4_report.md Secret

Created October 30, 2023 22:53
Show Gist options
  • Save greywolf12/1195510fb0aa329afa843fefa468777e to your computer and use it in GitHub Desktop.
Save greywolf12/1195510fb0aa329afa843fefa468777e to your computer and use it in GitHub Desktop.
Download ZIP
EnduracoinToken v4 Security Audit Report
Raw
Enduracoin_reaudit_4_report.md

EnduracoinToken v4 Security Audit Report

1. Summary

EnduracoinToken smart contract security audit report performed by GreyWolf

2. In scope

Commit 512d6d7d3392386ef52ee8e71ae830a24ffa6f52

  • EnduracoinToken.sol
  • EnduracoinValue.sol
  • ManageApprovers.sol
  • StringHelpers.sol
  • ChangeRequests.sol

3. Findings

In total, 2 issues were reported, including:

  • 0 high severity issues.

  • 1 medium severity issue.

  • 1 low severity issue.

In total, 2 notes were reported, including:

  • 1 minor observation.

  • 1 owner privileges.

3.1. Token functionality may be blocked for all users for voting period

Severity: medium

Description

All major functions (transfer, approve, transferFrom, etc) of EnduracoinToken have modifier requiresMultiSig(). This modifier apply checking of vote results if pendingChange.addrValueToChange == owner(). It means that any approver who create newChangeRequest with parameter addressToUse equal to owner address will block all tokens functionality until voting will be finished.

Code snipped

3.2. Owner privileges

Severity: owner privileges

Description

50 Billion Enduracoin will be pre-minted to the owner's wallet. If tokens are burnt, the owner has the right to mint new tokens up to 50 Billion in total supply.

3.3. The transfer ownership does not remove old address from approver list

Severity: low

Description

In the function execChangeRequest when ChangeType is transferOwnership and if new owner address already added to approver list, the old address will not be removed from approvers. Only if new owner isn't approver it will be added to the list and old address will be removed.

Code snipped

Recommendation

Rewrite this part: https://github.com/CallistoSecurity/EnduracoinToken/blob/512d6d7d3392386ef52ee8e71ae830a24ffa6f52/v4/EnduracoinToken.sol#L44-L53

like this:

            else if(pendingChangeType == uint8(ChangeType.transferOwnership)) { 
                if(validateApproverExists(owner())) 
                    _removeApprover(owner());
                if(!validateApproverExists(pendingChange.addrValueToChange)) 
                    _setApprover(pendingChange.addrValueToChange);
                transferOwnership(pendingChange.addrValueToChange); 
            }

3.4. Multiple minor observation

Severity: minor observation

Description

  1. The modifier requiresMultiSig restrict owner to call function until voting is finished, but allow anybody else to call function without restriction. Therefore, in the context of the contract EnduracoinValue it does not make any sense and can be removed or replaced by the modifier onlyApprovers.
  1. The getPendingChangeRequest() is a view function, so does not require onlyApprovers modifier.

  1. Owner can't create change request and must be the last approver in voting. If owner vote earlier the change request can't be approved and all others approver should vote against it to be able create new request. It's not a security issue, but may cause inconvenient.

  2. In the EnduracoinValue contract the comment say: "LASTDAY_OF_YEAR21 represents the last day of year 21 from the start date.", but value 8030 = 22 * 365. So it's last day of year 22. Also, this number does not take into account years with 366 days.

  1. In the EnduracoinValue contract the currentDayInPeriod should be in range from 0 to 364. Therefore condition if(daysSinceStart > 365) should be changed to if(daysSinceStart >= 365). or can be removed at all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment