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

When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. For example, if a user sends 100 deflationary tokens (with a 10% transaction fee) to a contract, only 90 tokens actually arrived in the contract. However, the user can still withdraw 100 tokens from the contract, which causes the contract to lose 10 tokens in such a transaction.

Incompatibility With Deflationary Tokens

In extreme case, bridge fee could be a very large amount of value, which might cause unexpected loss to the project and users.

Lack of Specified Fee Range Restriction

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

Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and update the fee structure.

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

Furthermore, the following functions :
- `activateBrgHoldingIncentive()`
- `activateAssetIncentive()`
- `activateAddressExemption()`

Any compromise to the `admin` accounts may allow the hacker to take advantage of this authority and activate or deactivate certain fee settings.


Centralization Risk in feeController.sol

In the contract `bridge` the roles `_owner`, admin, registrar, asset manager and/or migrator have authority over the following functions:
- `pauseBrigde()`
- `updateSettings()`
- `updateController()`
- `updateDeployer()`
- `activeNativeAsset()`
- `addForiegnAsset()`
- `initialMigration()`
- `completeMigration()`
- `registerNativeMigration()`
- `registerForiegnMigration()`

Any compromise to the roles `_owner`, admin, registrar, asset manager and/or migrator accounts may allow the hacker to take advantage of this authority and update critical parameters.

Centralization Risk in bridge.sol

The given input is missing the check for the non-zero address.

Missing Input Validation

If a user sends more token to the bridge when calling send, the remaining token will be locked in the bridge instead of sending back to the user.

For example, if user send 1000 token A and set the amount to be 100, the remaining 900 token will be locked in the bridge.

Remaining token not send back to user

The return value of `transferFrom()` is not checked.

Missing Return Value Check for `transferFrom()`

`completeMigration` can be called as long as `initialMigration` is called. If there are some tokens that are not migrated yet, it might cause some issues.

`completeMigration` can be called before migration finishes

The linked `else if` and `else` blocks will never be reached because the `isAssetManager(assetAddress ,true)` statement would never return `false` (as it either returns `true` or it reverts).

Unreachable Code

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

Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and add admins at will.

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

Furthermore, another role called `admin` would let certain privileged users call the functions:
- `addRegistrar()`
- `addOracle()`
- `addValidator()`

Centralization Risk in controller.sol

The controller smart contract is implemented to basically track a list of `admin`, `registrars`, `oracle` and `validator` addresses. The OpenZeppelin libraries `AccessControl` and `AccessControlEnumerable` are well-tested solutions to implement this kind of logic.

Use of AccessControl or AccessControlEnumerable OpenZeppelin Libraries

The error message in `require(!isRegistrar[_registrar] , "already an oracle")` and `require(isRegistrar[_registrar] , "not an oracle")` do not describe the error correctly because it is not checking whether the parameter is an oracle or not. Similarly, the error message in `require(!isRegistrar[_validator] , "already an oracle")` and `require(isValidator[_validator] , "not an oracle")` do not describe the error correctly because it is again not checking whether the parameter is an oracle or not.

This inconsistency looks like a copy-paste error.

Incorrect Error Message

The indentation of some lines is inconsistent with others.

Furthermore, it was noted that the codebase was, in general, not following solidity's style guide: https://docs.soliditylang.org/en/latest/style-guide.html

Inconsistent Line Indentation

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 differing compiler version numbers. 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

The highlighted variables assigned in the constructor can declare with `Immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since will not be stored in storage. Still, values will directly insert the values into the runtime code.

Variable Declare as Immutable

The linked statements do not affect the functionality of the codebase and appear to be either leftovers from test code or older functionality.

Redundant Code Components

The linked variables could be declared as `constant` since these state variables are never modified.

Variables That Could Be Declared as `constant`

The function that affects the status of sensitive variables should be able to emit events as notifications.

Missing Emit Events

One or more declarations do not conform to the [Solidity style guide](https://docs.soliditylang.org/en/latest/style-guide.html) with regards to its naming convention.

Particularly:

- Contract `bridge` is not in `CapWords` on file `bridge.sol`:
```solidity=18
contract bridge{
```

- Contract `token` is not in `CapWords` on file `token.sol`:
```solidity=5
contract token is ERC20 , Ownable{
```

- Contract `WRAPPEDTOKEN` is not in `CapWords` on file `wrappedToken`:
```solidity=5
 contract WRAPPEDTOKEN is ERC20 ,Ownable {
```

- Contract `registryContract` is not in `CapWords` on file `registry.sol`:
```solidity=12
contract  registryContract is Ownable {
```

- Contract `feeController` is not in `CapWords` on file `feeController.sol`:
```solidity=7
contract feeController  {
```

- Contract `controller` is not in `CapWords` on file `controller.sol`:
```solidity=5
contract controller is Ownable{
```

- Variable `ValidationPercentage` is not in `camelCase` on file `settings.sol`:
```solidity=13
     uint256 public ValidationPercentage = minValidationPercentage;
```

- Contract `settings` is not in `CapWords` on file `settings.sol`:
```solidity=4
contract settings{
```

- Parameter `_ValidationPercentage` is not in `camelCase` on file `settings.sol`:
```solidity=43
    function setMinValidationPercentage(uint256 _ValidationPercentage) external{
```

Declaration Naming Convention

In the contract `WrappedDeployer` the address `bridge` has authority over the function shown in the diagram below.

Any compromise to the `bridge` account may allow the hacker to take advantage of this authority and deploy wrapped assets at will.

![](https://accelerator-tasks-prod.s3.amazonaws.com/08030d347a5742d49a5f8b8e6063c426/diagrams/centralization-WrappedDeployer-bridge.svg)

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

Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and update the `bridge` state variable.

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

Finally, the `_owner` and controller admin accounts would have authority over the `updateLossless()` function and update some `WrappedDeployer` settings.

Centralization Risk in deployer.sol

The first part of the `require` statement makes sure the `bridge` state variable is equal to the `msg.sender`. Thus the second part of the condition `bridge != address(0)` is always true.

Redundant Condition

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

Usage of Hardhat's Console

The **require** can be used to check for conditions and throw an exception if the condition is not met. It is better to provide a string message containing details about the error that will be passed back to the caller.

Missing Error Messages

In the contract `settings` the roles `_owner` and admin have authority over the following functions:
- `setApprovedToAdd()`
- `setMinValidationPercentage()`
- `setbrgToken()`
- `setminWithdrawableFee()`
- `setNetworkSupportedChains()`
- `updateNetworkFee()`
- `setRailOwnerFeeShare()`
- `setUpdatableAssetState()`
- `setOnlyOwnableRailState()`
- `setrailRegistrationFee()`
- `setFeeRemitanceAddress()`

Any compromise to the `_owner` or admins accounts may allow the hacker to take advantage of this authority and update all settings.

Centralization Risk in settings.sol

In the contract `WRAPPEDTOKEN`, the role `owner` has authority over the following functions:
- burn
- mint

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

Centralization Related Risks

All of the tokens are sent to the contract deployer when deploying the contract. This could be a centralization risk as the deployer can distribute the tokens without obtaining the consensus of the community.

Bridge Network

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

26

0

8

4

14