crytic / medusa

Parallelized, coverage-guided, mutational Solidity smart contract fuzzing, powered by go-ethereum
https://secure-contracts.com/program-analysis/medusa/docs/src/
GNU Affero General Public License v3.0
301 stars 40 forks source link

Medusa is counting coverage multiple times during the contract construction #484

Open ggrieco-tob opened 1 month ago

ggrieco-tob commented 1 month ago

Given the following contract:

contract C {
}

contract Test {
  C x;
  constructor() {
    new C();
  }

  function h() public {
    x = new C(); 
  }
}

You can run medusa like this:

$ medusa fuzz --compilation-target test.sol --target-contracts Test
...
⇾ fuzz: elapsed: 3s, calls: 97787 (32594/sec), seq/s: 324, coverage: 1854, corpus: 100, failures: 0/974, gas/s: 2321324861

While medusa correctly executes only the f function (you can see it on the coverage report), the coverage count is incremented up to 1.8k. However, echidna PC counting is around 10 times smaller:

$ echidna test.sol --contract Test --test-mode assertion --test-limit 10000000
...
Unique instructions: 157
0xalpharush commented 1 month ago

We aren't deduplicating by hash. It's possible the two code hashes have disjoint coverage so we can't just ignore a map if it's hash has already been seen. I do wonder if it makes sense to consider two different contracts with the same code as uniquely covered e.g. if the contract is only called under some circumstance, reaching that coverage is unique and not fungible

https://github.com/crytic/medusa/blob/fc59d39af0adb201fabc82aec8972dedfec25505/fuzzing/coverage/coverage_maps.go#L252-L262

ggrieco-tob commented 1 month ago

From the user perspective, the issue here is that with the current approach medusa thinks that it is exploring more, but in reality, it is not. But it keeps adding sequences into the corpus that are useless.

0xalpharush commented 1 month ago

It's not being added to the corpus AFAICT bc that uses the codehash.