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

All of Rat Alter contracts that carry game mechanics are upgradeable, this idea behind this is to enable the RatAlert DAO to agree upon and change parameters of the game if required,and the contracts of the project are deployed with `proxy`. Apart from the logic in the specific logic contract, the contracts deployed via proxies can add additional permission controls or other logic. Since the proxy contract is not in the audit scope, it will be treated as a black box and assumed functional correctness. However, there will be potential centralization risk in the proxy:

- The admin of the proxy contract has the authority to execute any delegate call.
- Proxy-admin does not make a reasonable configuration in any code file.
  
Any compromise to the `admin` account may allow the hacker to take advantage of this and users' assets may suffer loss.

Centralization Risk in Proxy-admin Configuration

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

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

---


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

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


---

In the contract `Food` the role `controllers` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/05f7316cbf184f01b57abee45ec67b17/diagrams/centralization-Food-controllers.svg)


---


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

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

---


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

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

---


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

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

In the contract `Claim` the role `controllers` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/05f7316cbf184f01b57abee45ec67b17/diagrams/centralization-Claim-controllers.svg)

---

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

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

In the contract `Character` the role `controllers` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/05f7316cbf184f01b57abee45ec67b17/diagrams/centralization-Character-controllers.svg)

---

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

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

In the contract `VRFConsumer` the role `vrfCoordinator` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/05f7316cbf184f01b57abee45ec67b17/diagrams/centralization-VRFConsumer-vrfCoordinator.svg)

---



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

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

---


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

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

In the contract `Paywall` the role `controllers` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/05f7316cbf184f01b57abee45ec67b17/diagrams/centralization-Paywall-controllers.svg)

---


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

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

In the contract `Mint` the role `controllers` has authority over the functions shown in the diagram below.

![](https://accelerator-tasks-prod.s3.amazonaws.com/05f7316cbf184f01b57abee45ec67b17/diagrams/centralization-Mint-controllers.svg)


---


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

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

---


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

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

---


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

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

---



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

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


---


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

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


Any compromise to the privileged account may allow the hacker to take advantage of this authority and update the sensitive settings and execute sensitive functions of the project.

Centralization Related Risks

The white paper describes the supply of CFOOD as **10,000,000**, while in the code this number is **100,000,000** (10 times larger).
```solidity =3
contract CasualFood is Food {
  constructor() Food("CasualFood", "CFOOD", 100,000,000 * 10 ** 18) {}
}
```

inconsistency with White Paper

There is a possibility of exception throwing in dao.transfer, which can make a dos attack.
```solidity = 3
    if (msg.value > 0) {
      dao.transfer(msg.value); // Transfer to Gnosis Safe
    }
```
Specifically, the normal function of character.sol can be affected by dao address.

**Exploit**

For example, we can make the dao contract unable to accept certain eth transfers, thus preventing the character.sol contract from performing some of the mint operations.

Optimizable Transfer Patterns

```solidity = 6
  function setVenues(address[] memory _venues) external onlyOwner {
    delete venues;
    for (uint i = 0; i < _venues.length; i++) {
      venues.push(_venues[i]);
    }
  }
```
For the venues in Character, the contract does not use a hashmap to store them, but an array of addresses. This is indeed convenient for off-chain applications to get venues information via eth_getStorageAt()[RPC]. However, this is not in line with the common development model of solidity, and will lead to various serious security problems when the project growth and Owner misuse, as follows.

**Possible Impact**

The growing venues array will lead to  GAS reaching the upper limit causing DOS risk, which may lead to ERC721 tokens not being transferred in RatAlter. And because the venues removal operation is not available, the cost of fixing the problem is extremely high
```solidity = 9
  function transferFrom(address from, address to, uint256 tokenId) public virtual override {
    bool wl = false;
    for (uint i = 0; i < venues.length; i++) {
      wl = wl || _msgSender() == venues[i];
    }
    if (!wl)
      require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721: transfer caller is not owner nor approved");
    _transfer(from, to, tokenId);
  }
```

Optimizable Data Structure

Users only need to use the native token mint ERC721 tokens at the beginning of the game and pay nothing beyond the transaction fee after that.
```solidity=61
  function requestRandomness(bytes32 _keyHash, uint256 _fee) internal returns (bytes32 requestId) {
    link.transferAndCall(vrfCoordinator, _fee, abi.encode(_keyHash, USER_SEED_PLACEHOLDER));
    uint256 vRFSeed = makeVRFInputSeed(_keyHash, USER_SEED_PLACEHOLDER, address(this), nonces[_keyHash]);
    nonces[_keyHash] = nonces[_keyHash] + 1;
    return makeRequestId(_keyHash, vRFSeed);
  }
```
However, any call to `requestRandomNumber()` consumes link tokens, which gives malicious users the possibility to attack the project owner.

A malicious user can consume the project owner's link tokens by using FFOOD tokens to generate ERC721 tokens or by pledging ERC721 tokens. And in the process, they can also harvest more FOOD tokens without paying anything other than transaction fees.

A malicious user may benefit from this process as follows.
1. DOS contracts that prevent them from performing claim operations while increasing the benefit of their own claims.
2. consume the project's LINK tokens, causing financial losses to the project.

Users Do Not Pay For $link Usage

The kitchen token is a credential provided to chefs to access LeStake as well as TheStakeHouse. However, these tokens are not stake in contracts when it is being used. It is only to check the user balance at the time of stakeChef and claimChef.
This leads to the following unintended actions that can be made.
1. User A mint a certain amount of $kitchen tokens first, and does a stakeChef operation.
2. User A transfers $kitchen to B
3. B performs the stakechef operation, and then transfers $kitchen to A
4. A performs claimchef operation in the future and transfers $kitchen to B
5. B performs the claimchef operation

In this way, a \$Kitchen is used twice in a time period, ultimately increasing the revenue of \$XFOOD tokens


Flawed $Kitchen token implementation

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. 

And there are numerous `checkReceived` checks in the ERC721 contract as well as in the ERC1155 contract, which can lead to re-entry problems and eventually to unpredictable logic errors.
```solidity = 1
require(tx.origin == _msgSender(), "EOA only");
```
The use of msg.sender==tx.orign does prevent some reentrant attacks, but we also see many places where this check is not present and can certainly cause reentrants, especially in the context of many functions that do not adhere to CEI principles. 
```solidity =4
    if (unstake) {
      character.safeTransferFrom(address(this), sender, tokenId, ""); // Send Chef back to owner 
      delete chefs[tokenId];
      totalChefsStaked --;
    } 
```

Reentrant attacks can take many forms, and it should to eliminate the possibility of reentrants in all functions, even if they sometimes do not appear to be harmful at first glance.


Potential Reentrancy Attack

Events are an important implementation for Ether to provide contract operation info to the off-chain and provide data for the off-chain monitoring facility.Events should be designed to ensure coverage enough as well as difficult collisions.

But in the rat alter we found the following problems with the event design:

1. The function that affects the status of sensitive variables should be able to emit events as notifications to all of the function of [Controllable.sol] [Character.sol].
- `addController()`
- `removeController()`
- `setVenues()`
...
2. The output event information should give a more complete picture of the chain, There should be no ambiguity about different executions but outputting the same event.（That is, a collision occurs）
```solidity = 10
  function _stakeRat(address account, uint256 tokenId) internal whenNotPaused {
    ...
    emit TokenStaked(tokenId, account, foodTokensPerRat);//Can't tell if it's chefs or rats.
  }
  function _stakeChef(address account, uint256 tokenId) internal whenNotPaused {
    ...
    emit TokenStaked(tokenId, account, block.timestamp);//Can't tell if it's chefs or rats.
  }
```



Optimizable Emit Events Design

In the following code snippet:
```solidity
  function handle(address sender, uint8 amount, uint256 msgValue, uint16 minted, uint256 maxTokens, uint256 gen0Tokens) external onlyController returns (int8 boost) {
    ...
    if (totalCost > 0) fastFood.burn(sender, totalCost);
  }
```
It is very unreasonable to use burn mechanism in a Receive token function that has ERC20Capped. It would cause the following security risk.
1. **causing a severe shortage of token supply.** It may cause the project to run out of FFood or CFood during its lifetime, causing some of the project's functions to fail, such as the mint operation of ERC721 tokens.
2. **Amplifies the impact of centralization issues.** The burn mechanism is a high authority operation that does not require the authority of the authenticated token owner. This operation is given to all Controllers, which also contain Upgradeable contracts [Paywall, LeStake, etc.], and the Upgradability of these contracts may further aggravate the centralization problem of the project.



Receive Token by `burn()`

In the venues contract running the ERC721 token transfer, token owner authentication is implemented by venues contract, and for ERC721 contract it is not appropriate to skip the _isApprovedOrOwner() check for transactions sent from venues. This implementation exacerbates the project coupling level and makes the project security risk higher.
This increases the degree of project centrality.

Bad ERC721 Token Stake Method

```solidity
  function _mintFoodToken(address sender, uint256 amount) internal override {
    foodToken.mint(sender, amount);
  }
```

It is dangerous to use mint mechanism in a Payment function. It may cause the following results.
1. **The risk of breaking the economic model of the FOOD token as described in the white paper**. This may further result in users not being able to claim their ERC721token.

4. **Amplifies the impact of centralization issues.** The mint mechanism is a high authority operation that does not require the authority of the authenticated token owner. This operation is given to all Controllers, which also contain Upgradeable contracts, and the Upgradability of these contracts may further aggravate the centralization problem of the project.



Payment token by mint()

Be aware of the risk of fullfillClaimMany reaching the gas limit due to too many tokenIDs, which will lead to revert of fullfillClaimMany transactions. The same problem can occur in all functions that call requestRandomNumber such as mint() in Character.
``` solidity
  function mint(uint8 amount, bool stake) external payable whenNotPaused {
    ...
    theMint.requestRandomNumber(_msgSender(), amount, stake, boost);
  }
```
``` solidity
  function claimMany(uint16[] calldata tokenIds, bool unstake) external virtual payable whenNotPaused {
    ...
    bytes32 requestId = claim.requestRandomNumber(_msgSender(), tokenIds, unstake);
    ...
  }
  function fulfillClaimMany(IClaim.VRFStruct memory v, uint256 randomness) external virtual whenNotPaused {
    //spend many gas here
    require(msg.sender == address(claim), "Only Claim can fulfill");
    require(claimRequests[v.requestId].length > 0, "Claim request not found");

    uint16[] memory tokenIds = claimRequests[v.requestId];
    delete claimRequests[v.requestId];

    uint256 owed = 0;
    for (uint i = 0; i < tokenIds.length; i++) {
      uint256 randomVal = uint256(keccak256(abi.encode(randomness, i)));
      bool space = _checkSpace(v.sender, 0);
      if (isChef(tokenIds[i]))
        owed += _claimChef(tokenIds[i], v.sender, !space || v.unstake, !space, randomVal);
      else
        owed += _claimRat(tokenIds[i], v.sender, v.unstake, !space, randomVal);
      for (uint j = 0; j < stakers[v.sender].length; j++) {
        if (stakers[v.sender][j] == tokenIds[i]) {
          stakers[v.sender][j] = stakers[v.sender][stakers[v.sender].length - 1];
          stakers[v.sender].pop();
        }
      }
    }
    if (owed > 0) {
      _mintFoodToken(v.sender, owed);
    }
  }
```

Risk of transaction revert due to Gas reaching limit

`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

Traits is using the Base64 library implementation written by Brech Devos, which is undoubtedly a widely used and borrowed library, but as of 14 Sep 2021 Openzeppelin has implemented a base64 library code by borrowing from Brech Devos. It was equipped with more standardized documentation and testing efforts, as well as some optimizations.
For better maintenance and modularity later in the project. It is recommended to use Openzeppelin's Base64 library, as it has a larger community to maintain it and is equipped with better safeguards such as documentation and bug bounties.

Rat Alert

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

1

5

9