Sign Up for a Code Audit Today and Get 3 Months of Skynet for Free

In the contract `HordeLiquidityManagerV2` the role `_owner` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/ead589d0c62a4c7b9a3f990857ad34bb/diagrams/centralization-HordeLiquidityManagerV2-_owner.svg)

In the contract `HordeNodeManager` the role `_owner` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/ead589d0c62a4c7b9a3f990857ad34bb/diagrams/centralization-HordeNodeManager-_owner.svg)

In the contract `HordeV3` the role `_owner` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/ead589d0c62a4c7b9a3f990857ad34bb/diagrams/centralization-HordeV3-_owner.svg)

Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and have access to sensitive functionality.

Centralization Related Risks

A sandwich attack might happen when an attacker observes a transaction swapping tokens or adding liquidity without setting restrictions on slippage or minimum output amount. The attacker can manipulate the exchange rate by frontrunning (before the transaction being attacked) a transaction to purchase one of the assets and make profits by backrunning (after the transaction being attacked) a transaction to sell the asset.

The following functions are called without setting restrictions on slippage or minimum output amount, so transactions triggering these functions are vulnerable to sandwich attacks, especially when the input amount is large:

- `_addLiquidity()`
- `_swapTokensForBUSD()`
- `_swapBUSDForTokens()`

In the cases of the functions `_swapTokensForBUSD()`, `_swapBUSDForTokens()` there is no mechanism that prevents the `amountOutMin` parameter to be zero.

Potential Sandwich Attacks

The linked statements lack validation which can lead to unexpected behavior.

On the function `setAmountModifier()` from contract `HordeLiquidityManagerV2`, there is no validation with the range of the possible values. If the value `1_000` represents 1%, the maximum value should be `100_000`. The `permille` value needs to fall inside that range.

On the function `setFees()` from contract `HordeV3`, there is no validation to prevent that the sum of the fees is equal to `100`.

On the function `setThresholds()` from contract `HordeLiquidityManagerV2`, there is no mechanism that prevents the `upperBound` to be lower than the `lowerBand`, or the other way around.

On the function `setTokenSellFee()` from contract `HordeLiquidityManagerV2`, there is no mechanism that prevents the variable `tokenSellFee` from being bigger than `100`. This upper limit is given as the variable is used as a percentage.

Missing Input Validation

The linked statements do not handle the return value of an external call. This can lead to unexpected behavior.

On the contract `HordeV3`, the function `initialize()` makes an external call to Uniswap's `createPair()` function and then assigns the return value without validating it is not the zero address.

On the contract `HordeV3`, the function `_addLiquidity()` makes an external call to Uniswap's `addLiquidity()` function without veryfying the return value of the operation. Additionally, it calls the function `approve()` without verifying the result.

On the contract `HordeV3`, the function `_swapTokensForBUSD()` makes an external call to Uniswap's `swapExactTokensForTokensSupportingFeeOnTransferTokens()` function without verifying the return value of the operation.

On the contract `HordeLiquidityManagerV2`, the functions `_swapBUSDForTokens()` and `_swapTokensforBUSD()` make an external call to Uniswap's `swapExactTokensForTokensSupportingFeeOnTransferTokens()` function without verifying the return value of the operation. Additionally, it calls the function `approve()` without verifying the result.

On the contract `HordeLiquidityManagerV2`, the function `_addLiquidity()` makes an external call to Uniswap's `addLiquidity()` function without verifying the return value of the operation. Additionally, it calls the function `approve()` without verifying the result.

On the contract `HordeLiquidityManagerV2`, the functions `relocateBusdToTreasury()` and `relocateTokensToRewardPool()` make an external call to the function `approve()` without verifying the result.

Unhandled Return Value

The linked `tranfer()`/`transferFrom()` invocations do not check the return value of the function call which should yield a `true` result in case of a proper ERC-20 implementation.

Unchecked Value of ERC-20 `transfer()`/`transferFrom()` Call

The linked functions declare local variables that are only used once and don't improve the code's readability.

The function `getAllRewardAmountOf()` declares the variable `nodesCount` to store the total size of the array of nodes a user has.

The function `_getClaimRemainderInSeconds()` declares the variable `partial6HoursSinceLastClaimInSeconds` which is directly returned.

The function `_getNodeWithId()` uses the variables `found` and `validIndex` to check whether the index value is greater or equal than zero. This can be replaced by using the `require` statement to check if `index >= 0`. If the require suffices, the function can directly return `nodes[uint256(index)]`.

The function `equalizeBuybackCalculator()` from contract `HordeLiquidityManagerV2` has visibility of `internal` and it is only called by the function `equalizeBuy()` which supplies the value `priceBalancerLowerThreshold`. Given that the function is not called elsewhere with a different value, the parameter can be removed.

Unnecessary Variables or Parameters

Addresses should be checked before assignment or external call to make sure they are not zero addresses.

Missing Zero Address Validation

The contracts are serving as the underlying entity to interact with third party Uniswap protocol. The scope of the audit treats 3rd party entities as black boxes and assume their functional correctness. However, in the real world, 3rd parties can be compromised and this may lead to lost or stolen assets. In addition, upgrades of 3rd parties can possibly create severe impacts, such as increasing fees of 3rd parties, migrating to new LP pools, etc.

Third Party Dependencies

There are typos in the code:

- "periode" should be "period".
- "CLAIME" should be "CLAIM".
- "tokensIntoLiqudity" should be "tokensIntoLiquidity".

Additionally, looking into the project's [documentation](https://horde.games/landing/pdf/soulspaper.pdf?v=003), we found a typo on page 6, point 2. Where "protocl" should be "protocol".

Typos

The function `_distributeNodeCreationFees()` from contract `HordeV3` makes two calls to the function `_swapTokensForBUSD()`. This internal function, in turn, makes an external call to the Uniswap protocol.

A similar scenario happens with the function `swapHordeForBusd()` from contract `HordeLiquidityManagerV2`.

The consecutive calls can be merged into one by swapping only once the total amounts of tokens to be converted to BUSD, which is `treasuryTokens` and `hordePoolTokens`.

Then, by multiplying by the corresponding percentage, the function can manually transfer the BUSD tokens to the respective addresses.

Double Call To `_swapTokensForBUSD()`

The functions `_updateNodeAfterCashout()` and `_updateAllNodesAfterCashout()` from contract `HordeNodeManager` share a lot of functionality.

Something similar occurs with the functions `equalizeBuybackCalculator()` and `equalizeSelloffCalculator()` from contract `HordeLiquidityManagerV2`.

This shared functionality can be moved to an `internal` or `private` function to improve the code's readability.

Duplicated Code

The contract has unlocked compiler version. An unlocked compiler version in the source code of the contract permits the user to compile it at or above a particular version. This, in turn, leads to differences in the generated bytecode between compilations due to different compiler versions. This can lead to an ambiguity when debugging as compiler specific bugs may occur in the codebase that would be hard to identify over a span of multiple compiler versions rather than a specific one.

Unlocked Compiler Version

There should always be events emitted in the sensitive functions that are controlled by centralization roles.

Missing Emit Events

`public` functions that are never called by the contract could be declared as `external`. `external` functions are more efficient than `public` functions.

Improper Usage of `public` and `external` Type

Flash loans are a way to borrow large amounts of money for a certain fee. The requirement is that the loans need to be returned within the same transaction in a block. If not, the transaction will be reverted. 

An attacker can use the borrowed money as the initial funds for an exploit to enlarge the profit and/or manipulate the token price in the decentralized exchanges. 

We find that the function `getTokenPrice()` from `HordeLiquidityManagerV2` rely on price calculations that are based on-chain, meaning that they would be susceptible to flash-loan attacks by manipulating the price of given pairs to the attacker's benefit. 

Potential Flashloan Attack

On the function `init()` from contract `HordeLiqidityManager` there is a comment that states that the value of the state variable `_antiWhaleAmount` should be greater than `antiWhaleAmount` from the contract `HordeV3`.

Although this value is greater inside the `init()` function, there is no mechanism that prevents assigning a lower value via the `setAntiWhale()` function.

`_antiWhaleAmount` Can Be Lower Than `antiWhaleAmount` From `HordeV3`

The function `distributeTokenAmount` from contract `HordeLiquidityManagerV2` doesn't have a return value.

This makes it impossible at first glance to know if the function made the transactions or not.

Missing Return Value

The contract `HordeLiquidityManagerV2` should inherit from `IHordeLiquidityManager`.

Contract `HordeLiquidityManagerV2` Should Inherit From `IHordeLiquidityManager`

The contract `HordeLiquidityManager` uses the console contract from hardhat, which is meant to be used for testing purposes.

Usage of Hardhat's Console

The function `CreateNodeWithPromoToken()` is vulnerable to a reentrancy attack.

A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. 

If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects. 

In this case, the call to the external function `promoToken.transferFrom()` could lead to the `promoToken` contract to, in turn, call the function `CreateNodeWithPromoToken()` again and, as the value on the mapping `_promoClaimed` hasn't been updated yet, it will suffice the `require` statement and proceed in the creation of the node.

Although the contract `promoToken` is supposed to be safe by the team, given that it can change at any time with the use of the function `setPromoToken()`, if set to a malicious contract, a bad actor could receive an infinite amount of promotion nodes via exploiting this function.

Potential Reentrancy Attack

The functions `createNode()` and `calculateRewardForNode()` are declared as `internal` but the name doesn't start with `_`.

Internal Function Names Should Start with `_`

The function `_getRewardOf()` from contract `HordeNodeManager` gives the intuition that it will return the reward based on a parameter but instead it returns the fixed value of `rewardPerEvery6HoursOriginalNode`.

This naming can confuse future readers of the contract.

Unclear Function Naming `_getRewardOf()`

The function `CreateNodeWithPromoToken()` uses `msg.sender` to retrieve the sender of the transaction instead of using the function `_msgSender()`.

Given that the function is inherited from the contract `ContextUpgradeable`, the return value will be the same, but there will be consistency across the contract.

Usage of `msg.sender` instead of `_msgSender()`

The function `CreateNodeWithPromoToken()` from contract `HordeNodeManager` doesn't follow the Naming Convention as it starts with a capitalized letter.

Function Name Starts With Capitalized Latter

The linked magic numbers should be set as `constant` and `internal` contract-level variables with a self-explanatory variable name as well as accompanying comments when necessary. This type of declaration is functionally equivalent to the current implementation as `constant` variables that are `internal` or `private` are simply replaced in the codebase with their literal value.

Magic Numbers

The mapping `_promoClaimed` is declared to be of types `(address, uint32)` and is only used in the function `CreateNodeWithPromoToken()` to check if the user has already received a promotion node.

As the usage of the `uint32` is to check whether the value is different from zero, the mapping's value type `uint32` can be replaced with `bool`.

This change will save `31` bytes per item in the mapping as the boolean values only take `1` byte rather than the `32` used by `uint32`.

Mapping `_promoClaimed` Size Can Be Reduced

The function `distributeNodeCreationFees()` has the ability to distribute all of the contract's tokens to the addresses `treasuryPool`, `rewardPool` and `hordePool`.

Given that the owner can change all of the above addresses, this scenario can lead the owner to extract all the funds into an arbitrary address.

It is not clear why this function has the `public` modifier as by the context it lives in, it should be only called after a node has been purchased.

Centralization Risk: Owner Can Extract All Contract Funds via `distributeNodeCreationFees()`

The contract `HordeV3` has an "antiwhale" mechanism that consists of limiting the number of tokens that can be bought on a single transaction.

This mechanism can be easily bypassed by a user who has multiple wallets or by a user who makes consecutive transactions.

Ineffective Anti-Whale Mechanism

The function `_beforeTokenTransfer()` has an `if` statement which, to suffice, needs the `tradingEnabled` to be false and the `from` address to be set as true on the `automatedMarketMakerPairs` mapping.

If these two conditions suffice, then the `require` statement needs the `tradingEnabled`to to be true to suffice.

Thus, the `require` statement will never suffice.

Inconsistent Logic

The linked error messages are not clear.

- The message "Wallet is blacklisted" doesn't explain which address is the one blacklisted.
- The message "You are not permitted to swap buy more than #antiWhale tokens" uses both "swap" and "buy".

Unclear Error Messages

The function `_addLiquidity()` doesn't check if the parameters `tokenAmount` or `busdAmount` are above zero before executing. This could cause gas costs for each function call.

Missing Zero Amount Check

The contract `HordeV3` has a state variable `swapping` which is only used by the function `_distributeNodeCreationFees()`.

This function can only be called by the functions `payForNode()` and `distributeNodeCreationFees()`. Each of these functions has a modifier that restricts who can call them.

Therefore, there is no risk of reentrancy on the function `_distributeNodeCreationFees()`. Thus, the modifier is not needed.

Horde

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

32

0

8

1

23