Bug Hunting with Crytic

Crytic, our Github app for discovering smart contract flaws, is kind of a big deal: It detects security issues without human intervention, providing continuous assurance while you work and securing your codebase before deployment.

Crytic finds many bugs no other tools can detect, including some that are not widely known. Right now, Crytic has 90+ detectors, and we’re continuously adding new checks and improving existing ones. It runs these bug and optimization detectors on every commit, and evaluates custom security properties that you add yourself too!

Today, we’re sharing twelve issues in major projects that were found solely by Crytic, including some of high severity.

Here are the issues we discovered. Read on for where we found them:

  1. Unused return value can allow free token minting
  2. registerVerificationKey always returns empty bytes
  3. MerkleTree.treeHeight and MerkleTree.zero can be constant
  4. Modifiers can return the default value
  5. Dangerous strict equality allows the contract to be trapped
  6. ABI encodePacked collision
  7. Missing inheritance is error-prone
  8. Msg.value is used two times in fundWithAward
  9. Reentrancy might lead to theft of tokens
  10. [Pending acknowledgement]
  11. [Pending acknowledgement]
  12. [Pending acknowledgement]

Ernst & Young Nightfall

Crytic found three bugs in E&Y’s Nightfall project, including one critical vulnerability that could allow anyone to mint free tokens.

Issue 1: Unused return value can allow minting free tokens


FTokenShield.mint does not check the return value of transferFrom. If FTokenShield is used with a token that does not revert in case of incorrect transfers, and only returns false (e.g., BAT), anyone can mint free commitments—and an attacker can mint tokens for free.

A similar issue with less impact occurs in FTokenShield.burn: here fToken.transfer is not checked.


Check the return value of transferFrom and transfer.

Crytic report

Issue 2: registerVerificationKey always returns empty bytes


FTokenShield.registerVerificationKey and NFTokenShield.registerVerificationKey return an empty bytes32. It is unclear what the correct returned value should be. This might lead to unexpected behavior for third parties and contracts.


Consider either returning a value, or removing the return value from the signature.

Crytic report

Issue 3: MerkleTree.treeHeight and MerkleTree.zero can be constant


treeHeight and zero can be declared constant in MerkleTree.sol to allow the compiler to optimize this code.


State variables that never change can be declared constant to save gas.

Crytic report


Crytic found one unusual issue in DeFiStrategies: The lack of placeholder execution in a modifier leads the caller function to return the default value. Additionally, Crytic found an issue related to strict equality on the return of a balanceOf call.

Issue 4: Modifiers can return the default value


The SuperSaverZap.onlyInEmergency() and SuperSaverZap.stopInEmergency() modifiers do not revert in case of invalid access. If a modifier does not execute or revert, the execution of the function will return the default value, which can be misleading for the caller.


Replace the if() condition by a require in both modifiers.

Crytic report

Issue 5: Dangerous strict equality allows the contract to be trapped


ERC20toUniPoolZapV1_General.addLiquidity has strict equality on the _UniSwapExchangeContractAddress balance. This behavior might allow an attacker to trap the contract by sending tokens to _UniSwapExchangeContractAddress.


Change == to <= in the comparison.

Crytic report


Crytic found another issue that is not well known in DOSNetwork: If abi.encodedPacked is called with multiple dynamic arguments, it can return the same value with different arguments.

Issue 6: ABI encodePacked Collision


DOSProxy uses the encodePacked Solidity function with two consecutive strings (dataSource and selector): eth-contracts/contracts/DOSProxy.sol.

This schema is vulnerable to a collision, where two calls with a different dataSource and selector can result in the same queryId (see the Solidity documentation for more information).


Do not use more than one dynamic type in encodePacked, and consider hashing both dataSource and selector with keccak256 first.

Crytic Report


Crytic found an architectural issue in the Baseline Protocol, among others: A contract implementing an interface did not inherit from it.

Issue 7: Missing inheritance is error-prone


Shield is an implementation of ERC1820ImplementerInterface, but it does not inherit from the interface. This behavior is error-prone and might prevent the implementation or the interface from updating correctly.


Inherit Shield from ERC1820ImplementerInterface.

Crytic report


Crytic found another unusual issue in EthKidsthis.balance includes the amount of the current transaction (msg.value), which might lead to incorrect value computation.

Issue 8: Msg.value is used two times in fundWithAward


The use of this.balance in fundWithAward does not account for the value already added by msg.value. As a result, the price computation is incorrect.

fundWithAward computes the token amount by calling calculateReward with msg.value:

function fundWithAward(address payable _donor) public payable onlyWhitelisted {
   uint256 _tokenAmount = calculateReward(msg.value, _donor);

calculateReward calls calculatePurchaseReturn, where _reserveBalance is this.balance and _depositAmount is msg.value:

  return bondingCurveFormula.calculatePurchaseReturn(_tokenSupply, _tokenBalance, address(this).balance, _ethAmount);

In calculatePurchaseReturnn, baseN is then computed by adding _depositAmount (msg.value) to _reserveAmount (this.balance):

uint256 baseN = _depositAmount.add(_reserveBalance);

msg.value is already present in this.balance. For example, if this.balance is 10 ether before the transaction, and msg.value is 1 eth, this.balance will be 11 ether during the transaction). As a result, msg.value is used two times, and calculatePurchaseReturn is incorrectly computed.


Change the price computation so that _reserveBalance does not include the amount sent in the transaction.

Crytic report


Finally, Crytic found a well-known issue in HQ20: reentrancy. This reentrancy is interesting as it occurs if the contract is used with a token with callback capabilities (such as ERC777). This is similar to the recent uniswap and lendf.me hacks.

Issue 9: Reentrancy might lead to theft of tokens


Classifieds calls transferFrom on external tokens without following the check-effects-interaction pattern. This leads to reentrancy that can be exploited by an attacker if the destination token has a callback mechanism (e.g., an ERC777 token).

There are two methods with reentrancy issues:


Follow the check-after-effect pattern.

Crytic report

Start using Crytic today!

Crytic can save your codebase from critical flaws and help you design safer code. What’s not to like?

Sign up for Crytic today. Got questions? Join our Slack channel (#crytic) or follow @CryticCi on Twitter.

One thought on “Bug Hunting with Crytic

  1. Pingback: Upgradeable contracts made safer with Crytic | Trail of Bits Blog

Leave a Reply