Gondi Zenith
Findings & Analysis Report

2024-09-27

Table of contents

Overview

About C4

Code4rena (C4) is an open organization consisting of security researchers, auditors, developers, and individuals with domain expertise in smart contracts.

A C4 Zenith Audit is an event where elite tier Code4rena contributors, commonly referred to as wardens, reviews, audits and analyzes smart contract logic in exchange for a bounty provided by sponsoring projects.

During the Zenith audit outlined in this document, C4 conducted an analysis of the Gondi smart contract system written in Solidity. The audit took place between Sept 10 - Sept 12, 2024.

Wardens

2 Warden contributed to Gondi:

  1. 0x1771
  2. peakbolt

Final report assembled by Sentinel

Summary

The C4 Pro League analysis yielded 2 HIGH and 2 LOW risk severity vulnerabilities.

Scope

The source code was delivered to Code4rena in a private Git repository.

Severity Criteria

C4 assesses the severity of disclosed vulnerabilities based on three primary risk categories: high, medium, and low/non-critical.

High-level considerations for vulnerabilities span the following key areas when conducting assessments:

  • Malicious Input Handling
  • Escalation of privileges
  • Arithmetic
  • Gas use

For more information regarding the severity criteria referenced throughout the submission review process, please refer to the documentation provided on the C4 website, specifically our section on Severity Categorization.


High Risk Findings (2)

Votes for the veGondiToken should be zero after lock expiry

Context:

Description:

Users can lock their Gondi token for specific number of weeks in exchange for veGondiToken. The votes for each address are accounted in getVotesPerAddress[].

However, getVotesPerAddress remains the same even after the veToken lock expires. That means the token owner has no incentive to extendLock() when it expires, since the votes are unchanged.

This issue allows the owner to retain the votes even after lock expiry and still be able to burn/redeem the Gondi tokens anytime.

The votes for the veToken should instead be zero when the veToken is no longer locked.

    function _getVotingUnits(address account) internal view override returns (uint256) {
        return getVotesPerAddress[account]; 
    }

Recommendation:

This can be fixed by ensuring the delegated votes becomes zero when the veToken lock expires.

Gondi:

Fixed with a new checkpoint system to account for new votes and expired votes - commit

C4 Zenith:

Fix reviewed

Missing update of delegated votes in extendLock()

Context:

Description:

VeGondiToken inherits the Votes abstract contract and utilizes the vote delegation logic for accounting of votes. To ensure the delegated votes are properly accounted, Votes._transferVotingUnits() is called during both mint() and burn(), via the internal function _update().

However, extendLock() fails to update the delegated votes as it is not using transferVotingUnits() to keep track of the votes.

This issue will cause the delegated votes to be incorrect when an extendLock() is performed.

The following test shows that getVotes(tokenId) does not match getVotes(address) after extendLock().

    function testExtendLockIssue() public {
        uint256 lockup = 5;

        vm.startPrank(_user);
        _veGondiToken.delegate(_user);
        uint256 tokenId = _veGondiToken.mint(_user, _amount, lockup);

        console.log("unlockTime == ", _veGondiToken.getUnlockTime(tokenId));        
        console.log("getVotes(tokenId) == ", _veGondiToken.getVotes(tokenId));
        console.log("getVotes(address) == ", _veGondiToken.getVotes(_user));

        vm.warp(7 days + 1);
        _veGondiToken.extendLock(tokenId, lockup);
        vm.stopPrank();

        console.log("unlockTime == ", _veGondiToken.getUnlockTime(tokenId));        
        console.log("getVotes(tokenId) == ", _veGondiToken.getVotes(tokenId));
        console.log("getVotes(address) == ", _veGondiToken.getVotes(_user));
    }
[PASS] testExtendLockIssue() (gas: 382149)
Logs:
  unlockTime ==  5
  Votes(tokenId) ==  5000
  Votes(address) ==  5000
  unlockTime ==  10
  Votes(tokenId) ==  9000
  Votes(address) ==  5000

Recommendation:

This can be fixed by using transferVotingUnits() to update the votes for the account.

Gondi:

Fixed with PR-7

C4 Zenith:

Fix reviewed

Low Risk Findings (2)

Incorrect event emitted for transferOwnership()

Context:

Description:

TwoStepOwned.transferOwnership() emits an event when the ownership is successfully transferred from previous owner to new owner.

As owner has been set to newOwner, emit OwnershipTransferred will not reflect the previous owner in the event.

    function transferOwnership(address newOwner) public override onlyOwner {
        if (pendingOwnerTime + MIN_WAIT_TIME > block.timestamp) {
            revert TooSoonError();
        }
        if (pendingOwner != newOwner) {
            revert InvalidInputError();
        }
        owner = newOwner;
        pendingOwner = address(0);
        pendingOwnerTime = type(uint256).max;

>>>     emit OwnershipTransferred(owner, newOwner);
    }

Recommendation:

This can be fixed as follows:

- emit OwnershipTransferred(owner, newOwner);
+ emit OwnershipTransferred(msg.sender, newOwner);

Gondi:

Fixed with PR-10

C4 Zenith:

Fix reviewed.

VeGondiToken.tokenURI() is not compliant with ERC721

Context:

Description:

EIP-721 states that tokenURI() should throw an error if _tokenId is not a valid NFT.

/// @notice A distinct Uniform Resource Identifier (URI) for a given asset.
/// @dev Throws if `_tokenId` is not a valid NFT. URIs are defined in RFC
///  3986. The URI may point to a JSON file that conforms to the "ERC721
///  Metadata JSON Schema".
function tokenURI(uint256 _tokenId) external view returns (string);

However, VeGondiToken.tokenURI() fails to revert when an invalid _tokenId is provided.

function tokenURI(uint256 _id) public pure override returns (string memory) {
    return string.concat(URI, Strings.toString(_id));
}

Recommendation:

Suggest not to override this and stick to OZ’s ERC721 tokenURI(), which is compliant with EIP-721 as it will check that the NFT is owned. Refer to ERC721.sol#L88-L93

Gondi:

Fixed with PR-5

C4 Zenith:

Fix Reviewed

Disclosures

C4 is an open organization governed by participants in the community.

C4 audits incentivize the discovery of exploits, vulnerabilities, and bugs in smart contracts. Security researchers are rewarded at an increasing rate for finding higher-risk issues. Audit submissions are judged by a knowledgeable security researcher and solidity developer and disclosed to sponsoring developers. C4 does not conduct formal verification regarding the provided code but instead provides final verification.

C4 does not provide any guarantee or warranty regarding the security of this project. All smart contract software should be used at the sole risk and responsibility of users.