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

Investigate 0x00/empty string serialization #279

Open tuturu-tech opened 10 months ago

tuturu-tech commented 10 months ago

Medusa seems to save an empty string in the corpus for a "0x00" string input, which makes correctly parsing the corpus input values more difficult.

Example property:

    function check_specific_string(string memory provided) public {
        require(bytes(provided).length > 0);
        if (keccak256(bytes(provided)) == keccak256(bytes(hex"00"))) {
            assert(false);
        }
    }

Example corpus file:

[
 {
  "call": {
   "from": "0x0000000000000000000000000000000000030000",
   "to": "0xa647ff3c36cfab592509e13860ab8c4f28781a66",
   "nonce": 1,
   "value": "0x0",
   "gasLimit": 12500000,
   "gasPrice": "0x1",
   "gasFeeCap": "0x0",
   "gasTipCap": "0x0",
   "data": "0xe8f0d2db000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000", // <--- correctly encoded data for a non-empty string "\x00"
   "dataAbiValues": {
    "methodName": "check_specific_string",
    "inputValues": [
     "" // <--- seemingly empty string
    ]
   },
   "AccessList": null,
   "SkipAccountChecks": false
  },
  "blockNumberDelay": 1104,
  "blockTimestampDelay": 360624
 }
]
Xenomega commented 10 months ago

ABI values are serialized here, I'd check this out: https://github.com/crytic/medusa/blob/e471c52460ae9a37f5e168a96cbb0fbbe4db33a0/fuzzing/valuegeneration/abi_values.go#L534C13-L534C13

Solidity strings are just byte arrays with a given length, so \x00 should be retained, but it's probably lost in a []byte->string conversion in Go somewhere here, or not output correctly in the first place.

Note for any dev looking to tackle this in a PR: You'll want to add a unit test for this case to ensure this isn't ever broken again.

tuturu-tech commented 5 months ago

Contract and corpus for reproducing the issue: empty-string-reproduction.zip