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

Users can directly call all the public functions in collectors contract. However, according to the documentation，the user should call `cast` and the smart accounts would use NbnImplementationsM1 to cast the swap spell using the connector. 

Users can directly interact with functions in collectors 

In the contract `staking`, the role `owner` has the authority over the following function:
- setLockPeriodValues()
- removeLockPeriodValues()
- setMinlockPeriod()

Any compromise to the `owner` account may allow the hacker to take advantage of this, set/remove `LockPeriodValues` and set `minlockPeriod`. 

In the contract `Migrations`, the role `owner` has the authority over the following function:
- setCompleted()

Any compromise to the `owner` account may allow the hacker to take advantage of this and set `last_completed_migration`. 

In the contract `StorageOwners`, the role `owner` has the authority over the following function:
- changeStorageOracleStatus()
- activateStorageOracle()
- deactivateStorageOracle()

Any compromise to the `owner` account may allow the hacker to take advantage of this and change the status of `storageOracles[oracle]`. 

In the contract `StakingStorage`, the role `storageOracles` has the authority over the following function:
- CreateDepositorToDepositRecordIndexToRecordIDMapping()
- CreateStakingRecordMapping()
- UpdateStakingRecordMapping()
- CreateStakingRecordToDepositorMapping()

Any compromise to the `storageOracles` account may allow the hacker to take advantage of this, create `DepositorToDepositorRecordIndexMapping[depositor]`, create/update `StakingRecordMapping[StakingRecordId]` and create `depositorAddressMapping`. 

In the contract `StorageOwners`, the role `storageOracles` has the authority over the following function:
- reAssignStorageOracle()

Any compromise to the `storageOracles` account may allow the hacker to take advantage of this, deactivate `storageOracles[msg.sender]` and activate `storageOracles[newOracle]`. 

Centralization Risk

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. 

Check-effect-interaction Pattern Violation

The `cast` function uses the msg.value variable in a loop. While processing token payments involved a separate transferFrom call for each loop iteration, processing ETH payments simply checked whether msg.value was sufficient, which allows the attacker to reuse the same ETH multiple times.

ref: https://samczsun.com/two-rights-might-make-a-wrong/

msg.value Reuse

There are multiple typos in the aforementioned files. The `cToken` should be `vToken`.  

Typo

The input parameter `getId` is never used in function `harvest()`. 

Unused parameter

The function `withdraw` utilizes `muint` as the basis to withdraw lp token. However, the function `deposit()` doesn't call `setUint()` store the uint, the function `withdraw()` and function `harvest()` doesn't call `setUint()` to update the uint. 

Inconsistency between `getUint()` and `setUint()`

Since the function `withdraw` utilizes `recordID` as the basis to withdraw tokens, the finding is minor. However, the function `deposit()` calls `setUint()` to store the uint, while the function `withdraw()` doesn't call `setUint()` to update the uint and causes the inconsistency. 

The input parameter `getId` is never used in function `withdraw()`. However, according to the documentation, the function should utilize `getId` to retrieve the uniAmt from muint. 

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:

- `deposit()`
- `withdraw()`

Potential Sandwich Attacks

The function `sell` never uses `getId`. However, according to the documentation, the function should utilize `getId` to retrieve the `unitAmt` from `muint`. 

When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. As a result, an inconsistency in the amount will occur and the transaction may fail due to the validation checks.

Incompatibility With Deflationary Tokens(Swap)

If the connectors are deleted before the users withdraw all their money, the rest money won't be able to be retrieved. Any compromise to the `chief` and `master` accounts may allow a hacker to take advantage of this authority. 

Centralization Related Risks

`currentTimeStamp` is the value of block.timestamp, while `maturityDate` is the value of time period. Line 136 directly compares `currentTimeStamp` with `maturityDate`. 

Inconsistent Check

The implementation of function `changeStorageOracleStatus()` is the combination of functions `activateStorageOracle()` and `deactivateStorageOracle()`. 

Nubian

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

15

0

4

0

11