Token and Vesting - XHype

Prepared by:

HALBORN

Last Updated 06/25/2024

Date of Engagement by: June 24th, 2024 - June 24th, 2024

Summary

100% of all REPORTED Findings have been addressed

All findings

Critical

High

Medium

Low

Informational

1. Introduction
2. Assessment summary

2.1 Key findings and recommendations

3. Test approach and methodology
4. Risk methodology
5. Scope
6. Assessment summary & findings overview
7. Findings & Tech Details

7.1 Getter reverts for holders without vesting
7.2 Missing vesting schedule timings checks
7.3 Empty data return
7.4 Vesting schedule removal
7.5 Unnecessary payable declaration
7.6 Missing period verification

1. Introduction

xHype engaged our security analysis team to conduct a comprehensive security audit of their smart contract ecosystem. The primary aim was to meticulously assess the security architecture of the smart contracts to pinpoint vulnerabilities, evaluate existing security protocols, and offer actionable insights to bolster security and operational efficacy of their smart contract framework. Our assessment was strictly confined to the smart contracts provided, ensuring a focused and exhaustive analysis of their security features.

2. Assessment Summary

Our engagement with xHype spanned a 1 day period, during which we dedicated one full-time security engineer equipped with extensive experience in blockchain security, advanced penetration testing capabilities, and profound knowledge of various blockchain protocols. The objectives of this assessment were to:

- Verify the correct functionality of smart contract operations.

- Identify potential security vulnerabilities within the smart contracts.

2.1 Key Findings and Recommendations

Token Contract Usability Recommendations

To ensure the token contract operates purely as an ERC20 token and avoids any unintended interactions with vesting and dividend features, the following measures are recommended:

1. Disable swapEnabled: The swapEnabled feature should be disabled to prevent any swapping functionality.

Important: Call toggleSwapEnabled to disable swapEnabled as it is set to true by default.

2. Adjust buyFee and sellFee: Configure these fees as per your requirements.

3. Avoid setVestedWallet Function: This function should never be called to prevent triggering the internal vesting functionality, which has known issues.

4. Do Not Trust Dividend-Related view Functions: All view functions related to dividends and shares are unreliable and should not be used.

5. Set pointsPerShare to Zero: Ensure this value is zero to prevent users from withdrawing unintended funds via withdrawDividends.

Vesting Contract Issues

1. Reverts for Holders Without Vesting ( getLastVestingScheduleForHolder ): This function reverts if the holder does not have any vesting schedules, due to an index out-of-bounds error.

2. Invalid Vesting Schedule Timings: There are no checks to ensure _start, _start + cliff, or start + _duration are in the future relative to block.timestamp, leading to invalid vesting schedules.

3. Empty Data Return in getVestingSchedule Functions: These functions may return empty data if the vesting schedule ID does not exist, leading to unintended behavior.

4. Vesting Schedule Removal in revoke: The revoke function does not remove the vesting schedule from vestingSchedulesIds array, potentially leading to issues with iterating over revoked schedules.

5. Unnecessary Payable Declaration in release: The beneficiary is unnecessarily declared as payable in the release function, despite no Ether transfer being involved.

6. Missing _slicePeriodSeconds Verification: There is no check to ensure _slicePeriodSeconds is not greater than the duration, potentially causing all tokens to be released only at the end of the vesting period.

Addressing these issues will significantly enhance the security, functionality, and user trust in XHP's smart contract ecosystem. Implementing the recommended changes will mitigate risks and align operations with the best security practices.

3. Test Approach and Methodology

Our testing strategy employed a blend of manual and automated techniques to ensure a thorough evaluation. While manual testing was pivotal for uncovering logical and implementation flaws, automated testing offered broad code coverage and rapid identification of common vulnerabilities. The testing process included:

- A detailed examination of the smart contracts' architecture and intended functionality.

- Comprehensive manual code reviews and walkthroughs.

- Functional and connectivity analysis utilizing tools like Solgraph.

- Customized script-based manual testing and testnet deployment using Foundry.

This executive summary encapsulates the pivotal findings and recommendations from our security assessment of Renzo's smart contract ecosystem. By addressing the identified issues and implementing the recommended fixes, Renzo can significantly boost the security, reliability, and trustworthiness of its smart contract platform.

4. RISK METHODOLOGY

Every vulnerability and issue observed by Halborn is ranked based on two sets of Metrics and a Severity Coefficient. This system is inspired by the industry standard Common Vulnerability Scoring System.

The two Metric sets are: Exploitability and Impact. Exploitability captures the ease and technical means by which vulnerabilities can be exploited and Impact describes the consequences of a successful exploit.

The Severity Coefficients is designed to further refine the accuracy of the ranking with two factors: Reversibility and Scope. These capture the impact of the vulnerability on the environment as well as the number of users and smart contracts affected.

The final score is a value between 0-10 rounded up to 1 decimal place and 10 corresponding to the highest security risk. This provides an objective and accurate rating of the severity of security vulnerabilities in smart contracts.

The system is designed to assist in identifying and prioritizing vulnerabilities based on their level of risk to address the most critical issues in a timely manner.

4.1 EXPLOITABILITY

Attack Origin (AO):

Captures whether the attack requires compromising a specific account.

Attack Cost (AC):

Captures the cost of exploiting the vulnerability incurred by the attacker relative to sending a single transaction on the relevant blockchain. Includes but is not limited to financial and computational cost.

Attack Complexity (AX):

Describes the conditions beyond the attacker’s control that must exist in order to exploit the vulnerability. Includes but is not limited to macro situation, available third-party liquidity and regulatory challenges.

Metrics:

EXPLOITABILIY METRIC ( $m_e$ )	METRIC VALUE	NUMERICAL VALUE
Attack Origin (AO)	Arbitrary (AO:A) Specific (AO:S)	1 0.2
Attack Cost (AC)	Low (AC:L) Medium (AC:M) High (AC:H)	1 0.67 0.33
Attack Complexity (AX)	Low (AX:L) Medium (AX:M) High (AX:H)	1 0.67 0.33

Exploitability

E

is calculated using the following formula:

$E = \prod m_e$

4.2 IMPACT

Confidentiality (C):

Measures the impact to the confidentiality of the information resources managed by the contract due to a successfully exploited vulnerability. Confidentiality refers to limiting access to authorized users only.

Integrity (I):

Measures the impact to integrity of a successfully exploited vulnerability. Integrity refers to the trustworthiness and veracity of data stored and/or processed on-chain. Integrity impact directly affecting Deposit or Yield records is excluded.

Availability (A):

Measures the impact to the availability of the impacted component resulting from a successfully exploited vulnerability. This metric refers to smart contract features and functionality, not state. Availability impact directly affecting Deposit or Yield is excluded.

Deposit (D):

Measures the impact to the deposits made to the contract by either users or owners.

Yield (Y):

Measures the impact to the yield generated by the contract for either users or owners.

Metrics:

IMPACT METRIC ( $m_I$ )	METRIC VALUE	NUMERICAL VALUE
Confidentiality (C)	None (I:N) Low (I:L) Medium (I:M) High (I:H) Critical (I:C)	0 0.25 0.5 0.75 1
Integrity (I)	None (I:N) Low (I:L) Medium (I:M) High (I:H) Critical (I:C)	0 0.25 0.5 0.75 1
Availability (A)	None (A:N) Low (A:L) Medium (A:M) High (A:H) Critical (A:C)	0 0.25 0.5 0.75 1
Deposit (D)	None (D:N) Low (D:L) Medium (D:M) High (D:H) Critical (D:C)	0 0.25 0.5 0.75 1
Yield (Y)	None (Y:N) Low (Y:L) Medium (Y:M) High (Y:H) Critical (Y:C)	0 0.25 0.5 0.75 1

Impact

I

is calculated using the following formula:

$I = max(m_I) + \frac{\sum{m_I} - max(m_I)}{4}$

4.3 SEVERITY COEFFICIENT

Reversibility (R):

Describes the share of the exploited vulnerability effects that can be reversed. For upgradeable contracts, assume the contract private key is available.

Scope (S):

Captures whether a vulnerability in one vulnerable contract impacts resources in other contracts.

Metrics:

SEVERITY COEFFICIENT ( $C$ )	COEFFICIENT VALUE	NUMERICAL VALUE
Reversibility ( $r$ )	None (R:N) Partial (R:P) Full (R:F)	1 0.5 0.25
Scope ( $s$ )	Changed (S:C) Unchanged (S:U)	1.25 1

Severity Coefficient

C

is obtained by the following product:

$C = rs$

The Vulnerability Severity Score

S

is obtained by:

$S = min(10, EIC * 10)$

The score is rounded up to 1 decimal places.

Severity	Score Value Range
Critical	9 - 10
High	7 - 8.9
Medium	4.5 - 6.9
Low	2 - 4.4
Informational	0 - 1.9

5. SCOPE

Files and Repository

(a) Repository: 0x747747e47a48c669be384e0dfb248eee6ba04039

(b) Assessed Commit ID:

token/0x747747e47a48c669be384e0dfb248eee6ba04039
XHPVesting.sol (zip file)

Out-of-Scope:

Out-of-Scope: New features/implementations after the remediation commit IDs.

6. Assessment Summary & Findings Overview

Critical

High

Medium

Low

Informational

Security analysis	Risk level	Remediation Date
Getter reverts for holders without vesting	Low	Risk Accepted
Missing vesting schedule timings checks	Low	Risk Accepted
Empty data return	Low	Risk Accepted
Vesting schedule removal	Informational	Acknowledged
Unnecessary payable declaration	Informational	Acknowledged
Missing period verification	Informational	Acknowledged

7. Findings & Tech Details

7.1 Getter reverts for holders without vesting

// Low

Description

The function getLastVestingScheduleForHolder reverts if the holder does not have any vesting schedules because it attempts to access an index that is out of bounds. This happens due to the computation holdersVestingCount[holder] - 1, which results in an underflow and reverts the transaction if holdersVestingCount[holder] is zero.

BVSS

AO:A/AC:L/AX:L/C:N/I:L/A:N/D:N/Y:N/R:N/S:U (2.5)

Recommendation

Before attempting to retrieve the last vesting schedule for a holder, check if the holder has any vesting schedules. If holdersVestingCount[holder] is zero, the function should revert with a meaningful error message.

function getLastVestingScheduleForHolder(address holder)
    public
    view
    returns(VestingSchedule memory)
{
    require(holdersVestingCount[holder] > 0, "XHPVesting: holder has no vesting schedules");
    return vestingSchedules[computeVestingScheduleIdForAddressAndIndex(holder, holdersVestingCount[holder] - 1)];
}

Remediation Plan

RISK ACCEPTED: The XHype team accepted the risk of this finding.

7.2 Missing vesting schedule timings checks

// Low

Description

In the createVestingSchedule function, there are no checks to ensure that the _start, _start + cliff, or start + cliff + duration values are in the future relative to block.timestamp. This can result in creating vesting schedules that have already started, have their cliff in the past, or are already fully vested.

BVSS

AO:A/AC:L/AX:L/C:N/I:L/A:N/D:N/Y:N/R:N/S:U (2.5)

Recommendation

Add checks to ensure that _start, _start + cliff, or start + cliff + duration are greater than the current block.timestamp.

require(_start >= block.timestamp, "XHPVesting: start time must be in the future");
require(_start.add(_cliff) >= block.timestamp, "XHPVesting: cliff time must be in the future");
require(_start.add(_cliff).add(_duration) > block.timestamp, "XHPVesting: end time must be in the future");

Remediation Plan

RISK ACCEPTED: The XHype team accepted the risk of this finding.

7.3 Empty data return

// Low

Description

The functions getVestingSchedule and getVestingScheduleByAddressAndIndex may return empty data if the vesting schedule ID does not exist in the mapping. This could lead to unintended behavior if these functions are used without proper validation.

BVSS

AO:A/AC:L/AX:L/C:N/I:L/A:N/D:N/Y:N/R:N/S:U (2.5)

Recommendation

Add checks to ensure that the vesting schedule exists before returning the data. Revert with a meaningful error message if the vesting schedule does not exist.

function getVestingSchedule(bytes32 vestingScheduleId)
    public
    view
    returns(VestingSchedule memory)
{
    require(vestingSchedules[vestingScheduleId].beneficiary != address(0), "XHPVesting: vesting schedule does not exist");
    return vestingSchedules[vestingScheduleId];
}

Remediation Plan

RISK ACCEPTED: The XHype team accepted the risk of this finding.

7.4 Vesting schedule removal

// Informational

Description

The revoke function currently does not remove the vesting schedule from the vestingSchedulesIds array, which might be unnecessary if not required by the frontend or any UI.

Score

AO:A/AC:L/AX:L/C:N/I:N/A:N/D:N/Y:N/R:N/S:U (0.0)

Recommendation

Consider removing the vesting schedule from the vestingSchedulesIds array when it is revoked, to keep the array clean and avoid potential issues with iterating over revoked schedules.

Remediation Plan

ACKNOWLEDGED: The XHype team acknowledged this finding.

7.5 Unnecessary payable declaration

// Informational

Description

In the release function, the beneficiary is unnecessarily declared as payable, even though there is no Ether transfer involved. This is redundant and can be removed for clarity.

Score

AO:A/AC:L/AX:L/C:N/I:N/A:N/D:N/Y:N/R:N/S:U (0.0)

Recommendation

Remove the payable keyword from the beneficiary declaration in the release function.

Remediation Plan

ACKNOWLEDGED: The XHype team acknowledged this finding.

7.6 Missing period verification

// Informational

Description

In the createVestingSchedule function, there is no check to ensure that _slicePeriodSeconds is not greater than the vesting duration. If _slicePeriodSeconds is greater than the duration, the vestedAmount will always be zero until the full duration has passed, causing the tokens to be released only at the end of the vesting period. This behavior defeats the purpose of having periodic vesting slices.

Score

AO:A/AC:L/AX:L/C:N/I:N/A:N/D:N/Y:N/R:N/S:U (0.0)

Recommendation

Add a check to ensure that _slicePeriodSeconds is not greater than the duration when creating a vesting schedule. Additionally, consider setting an appropriate maximum value for _slicePeriodSeconds, such as 1 week or 1 day, depending on the desired vesting frequency.

Remediation Plan

ACKNOWLEDGED: The XHype team acknowledged this finding.

References

Halborn strongly recommends conducting a follow-up assessment of the project either within six months or immediately following any material changes to the codebase, whichever comes first. This approach is crucial for maintaining the project’s integrity and addressing potential vulnerabilities introduced by code modifications.

1. Introduction
2. Assessment summary

2.1 Key findings and recommendations

3. Test approach and methodology
4. Risk methodology
5. Scope
6. Assessment summary & findings overview
7. Findings & Tech Details

7.1 Getter reverts for holders without vesting
7.2 Missing vesting schedule timings checks
7.3 Empty data return
7.4 Vesting schedule removal
7.5 Unnecessary payable declaration
7.6 Missing period verification

// Download the full report

* Use Google Chrome for best results

** Check "Background Graphics" in the print settings if needed