bitcoinfuzz icon indicating copy to clipboard operation
bitcoinfuzz copied to clipboard

Add Target onion_legacy_decode

Open erickcestari opened this issue 1 month ago • 10 comments

I'll develop the fuzzing targets in two phases: starting with the legacy onion format, followed by the modern TLV-based onion format.

Legacy Onion Format

Custom Mutator Development

  • Integrate secp256k1 as a library within the custommutator directory for cryptographic operations
  • Implement a custom mutator that performs the following workflow:
    1. Decrypt the onion packet
    2. Mutate the raw payload data
    3. Re-encrypt the modified packet

Target Implementation

  • Create fuzzing targets for Core Lightning and LND implementations
  • Note: rust-lightning will be excluded from legacy onion testing as it doesn't support the legacy format

Related to #306

erickcestari avatar Nov 12 '25 17:11 erickcestari

Would it be worth reusing secp256k1 inside the Bitcoin folder instead of copying it from there to the custom mutator folder?

erickcestari avatar Nov 13 '25 12:11 erickcestari

Would it be worth reusing secp256k1 inside the Bitcoin folder instead of copying it from there to the custom mutator folder?

How would you reuse it? If it means to build the Bitcoin Core module, so I prefer to not use it. Also, using secp from Bitcoin module means that every update on Bitcoin Core's module would affect the custom mutators.

brunoerg avatar Nov 14 '25 11:11 brunoerg

How would you reuse it? If it means to build the Bitcoin Core module, so I prefer to not use it. Also, using secp from Bitcoin module means that every update on Bitcoin Core's module would affect the custom mutators.

By that, I mean compiling the secp256k1 library using the secp256k1 files in the Bitcoin folder and copying the compiled library to the custom mutator. Is it common for changes to be made to secp256k1 within bitcoin? If so, I agree that we should maintain it separately.

erickcestari avatar Nov 14 '25 12:11 erickcestari

Actually I will put this in draft for now, because I don't think that we need to create 2 different targets and custom mutator for decoding onions.

erickcestari avatar Nov 14 '25 13:11 erickcestari

Nice work. Do you have a link to the legacy format to help in reviewing this?

Actually I will put this in draft for now, because I don't think that we need to create 2 different targets and custom mutator for decoding onions.

Do you mean to have a single target that compares both legacy and regular format, or something else?

morehouse avatar Nov 14 '25 16:11 morehouse

Nice work. Do you have a link to the legacy format to help in reviewing this?

Thanks! There is a book mastering lightning network and the old bolt 04

https://github.com/rustyrussell/bolts/blob/bc86304b4b0af5fd5ce9d24f74e2ebbceb7e2730/04-onion-routing.md#legacy-hop_data-payload-format

Do you mean to have a single target that compares both legacy and regular format, or something else?

Yes. I mean that we don't actually need to separate them since they are very similar, and the fuzzer should be able to create valid inputs for both formats without the need for a custom mutator for each one.

I don't fully understand why this custom mutator currently only works well with legacy HopData payloads. This is because the custom mutator is essentially decrypting, encrypting and mutating the raw bytes inside the onion. So it should work for both payload formats.

Btw, the name 'legacy onion' is misleading (my mistake). The correct names are the 'Legacy HopData Payload Format' and the 'TLV Payload Format'.

erickcestari avatar Nov 14 '25 17:11 erickcestari

I don't fully understand why this custom mutator currently only works well with legacy HopData payloads. This is because the custom mutator is essentially decrypting, encrypting and mutating the raw bytes inside the onion. So it should work for both payload formats.

I suspect the mutator struggles with the length field of hop_payloads. In the legacy format it's always a single 0 byte, which is a value fuzzers like to choose already. For the TLV format, length actually needs to be a value between 2 and 1265 (I think), encoded as BigSize. That means the first byte needs to be 0xfd and then the next two bytes need to be a uint16 in the correct range. Choosing randomly, the fuzzer has only a 1/256 * 1263/65536 = 0.0075% chance of choosing a valid length.

We could try making the mutator smarter about the length: some percentage of the time we could choose a random value, but most of the time we could choose a length in the correct range.

morehouse avatar Nov 14 '25 17:11 morehouse

That means the first byte needs to be 0xfd and then the next two bytes need to be a uint16 in the correct range. Choosing randomly, the fuzzer has only a 1/256 * 1263/65536 = 0.0075% chance of choosing a valid length.

Makes sense! It also can be in this range right?

uint8(x)                if x < 0xfd

The values 0 and 1 aren’t valid, but any length from 2 up to 252 can be encoded this way, right?

erickcestari avatar Nov 14 '25 18:11 erickcestari

Yes, right!

morehouse avatar Nov 14 '25 20:11 morehouse

I added your idea to the custom mutator, as well as another that mutates the bytes within the BigSize range to increase the likelihood of generating a valid TLV. After running the fuzzer for some time, it generated a valid onion with the TLV payload!

erickcestari avatar Nov 15 '25 15:11 erickcestari