Opened 10 months ago

Last modified 5 weeks ago

#21509 accepted task

Fuzz v3 hidden services

Reported by: teor Owned by: nickm
Priority: High Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: fuzz, prop224, tor-hs
Cc: Actual Points:
Parent ID: Points: 2
Reviewer: Sponsor: SponsorR-can

Description (last modified by teor)

If we want the fuzzer to effectively fuzz v3 hidden services, we need to:

  • fuzz GET requests: #21476
  • fuzz POST requests: #21478
  • add v3 GET and POST requests to the fuzzing corpus
  • add tokens from v3 GET and POST requests as new fuzzing token lists
  • disable the encrypted connection check when fuzzing (we should do this for v2 services as well)
  • create a v3 descriptor fuzzer
  • add v3 descriptor examples to the fuzzing corpus
  • add tokens from v3 descriptors as a new fuzzing token list

Child Tickets

Attachments (1)

hs_descriptor.c.gcov (126.6 KB) - added by nickm 7 weeks ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 10 months ago by nickm

Sponsor: SponsorR-can

comment:2 Changed 10 months ago by nickm

Points: 2
Priority: MediumHigh

comment:3 Changed 10 months ago by dgoulet

Priority: HighVery High

Prioritize prop224 tickets for 031 milestone. They are all "Enhancement".

comment:4 Changed 8 months ago by dgoulet

Owner: set to dgoulet
Status: newaccepted

Taking those ticket for patches.

comment:5 Changed 8 months ago by dgoulet

Type: enhancementtask

More of a task, not really a feature.

comment:6 Changed 7 months ago by teor

Description: modified (diff)

comment:7 Changed 4 months ago by dgoulet

Milestone: Tor: 0.3.1.x-finalTor: 0.3.2.x-final

I've done some initial fuzzing for the descriptor encoding/decoding. More is needed for service and client in 032. Moving this out of 031 milestone.

comment:8 Changed 4 months ago by dgoulet

Status: acceptedneeds_revision

This has been merged upstream but then disabled because of a problem with how the HS API is used.

Commit 97347b11 adds basic fuzzing for descriptors and disabled in 5ef656e7

comment:9 Changed 3 months ago by dgoulet

Owner: changed from dgoulet to haxxpop
Status: needs_revisionassigned

comment:10 Changed 8 weeks ago by dgoulet

Owner: changed from haxxpop to dgoulet
Status: assignedaccepted

We need this for 032 alpha so I'm taking ownership.

comment:11 Changed 8 weeks ago by dgoulet

Status: acceptedneeds_review

See branch: bug21509_032_01

The branch reverts 5ef656e7 which disabled the fuzzing for v3 descriptor. Then, I've added a dummy subcredential that is mandatory for the decoding API.

This makes the decryption fail all the time of the encrypted layer but that is fine because this very basic fuzzing program only fuzz the plaintext part for now.

comment:12 Changed 8 weeks ago by nickm

merging to 0.3.2 and forward.

How hard would it be to get the decrypted part fuzzed too?

comment:13 in reply to:  12 Changed 8 weeks ago by dgoulet

Replying to nickm:

merging to 0.3.2 and forward.

How hard would it be to get the decrypted part fuzzed too?

A bit more work in mocking functions because lots of crypto checks are done (decrypt, MAC validation, signature...)

And we would need to expose the inners like desc_decrypt_all() or decrypt_desc_layer(). A bit of work but not that crazy I think.

comment:14 Changed 7 weeks ago by nickm

Owner: changed from dgoulet to nickm
Status: needs_reviewaccepted

I've got a fuzzer for the decrypted parts, but I should run it for a while before uploading.

comment:15 Changed 7 weeks ago by nickm

Status: acceptedneeds_review

Okay, the fuzzers didn't find anything. What do you think of hsdescv3_fuzz_more?

comment:16 Changed 7 weeks ago by dgoulet

Status: needs_reviewmerge_ready

Great addition!

This will allow us to test the decode_superencrypted() function but most of it is tokenize_string(). So a good next step would be to explicitly fuzz decode_intro_points() which happens if the super encrypted section is properly decrypted and decoded. See desc_decode_encrypted_v3()

Thanks!

comment:17 Changed 7 weeks ago by nickm

Priority: Very HighHigh
Status: merge_readyaccepted

merged to 0.3.2!

Sounds like you think there's more work, so putting this back in "accepted".

Changed 7 weeks ago by nickm

Attachment: hs_descriptor.c.gcov added

comment:18 Changed 7 weeks ago by nickm

I've attached the gcov output of running the fuzz_static_testcases.sh script on hs_descriptor.c with the current fuzzing corpus. Note that this doesn't actually fuzz -- it just shows us what our current corpus reaches. But it looks like we're at least getting inside decode_intro_points() a little? We should add some seed elements to the corpus that trigger more of it getting parsed, though.

comment:19 Changed 5 weeks ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.3.x-final
Note: See TracTickets for help on using tickets.