Write unit tests for security regressions

When we fix a security vulnerability like the ones in [ https://trac.torproject.org/projects/tor/wiki/TROVE TROVE], we sometimes forget to include a unit test.

Some security vulnerabilities are easy to test, such as parsing errors. We should write tests that fail with the bug, but succeed when it is fixed.

Here's an initial list of security fixes we can write unit tests for:

• #20894 (moved)
• #21018 (moved)
• #21278 (moved) (and #21450 (moved))

Let's open a child ticket of this ticket for each one.

changed milestone to %Tor: 0.3.1.x-final

added 029-backport 030-backport component::core tor/tor milestone::Tor: 0.3.1.x-final priority::medium resolution::fixed severity::normal status::closed test type::task labels

Can we just add the relevant fuzzing failure cases to tor-fuzzing-corpora?

Replying to nickm:

Can we just add the relevant fuzzing failure cases to tor-fuzzing-corpora?

That works too, as long as we run it often enough (I guess Google will do that for us?)

We should also add a case for #21471 (moved) (and a v3 HS descriptor GET and POST) to http.

The recommended way is to add the failing inputs to the corpus once the bug is fixed. This way oss-fuzz will regularly run these inputs.

We also recommend to have your own CI system run the inputs (not necessary with fuzzing), especially if you have pre-submit testing, to find the regressions earlier. oss-fuzz does not have any SLA regarding the turnaround time, and we can typically detect a bug within 24 hours after submission. It's very unlikely that we will report a regression earlier than 3-4 hours after the commit.

Having a separate unit test that detects the same bug is redundant to some extent, but still might be a good idea sometimes (especially if you do run unit tests and don't execute fuzz targets on their corpora before submit)

Trac:
Username: kcc

I added oss-fuzz/408 to our corpus repository with commit abd71a897d0618edb295bcfa93dc36899fc2b82c.

I opened a ticket (#21491 (moved)) to check our fuzzing corpora as part of our CI process.

(Teor, do you happen to have test files for any of the other cases that I could add?)

For #20894 (moved), the test case is: https://github.com/teor2345/tor/blob/fuzz-dir-sensitive-v2/src/test/fuzz/fuzz_dir_testcase/dir-header-read-beyond-buffer.case

Do you still have the #21018 (moved) test case? I think you found it with libfuzzer? (Or is it already in the corpus?)

For #21450 (moved), I think a unit test might be the easiest option, as we want something that differs on i386 and x86_64. (Are we fuzzing on i386, or only x86_64?)

Replying to teor:

We should also add a case for #21471 (moved) (and a v3 HS descriptor GET and POST) to http.

Oh, and #21471 (moved) will require a GET request for a v3 HS descriptor, and disabling the encrypted connection check in the function (or changing the mocked connection so it looks like it's encrypted).

Replying to teor:

Replying to teor:

We should also add a case for #21471 (moved) (and a v3 HS descriptor GET and POST) to http.

Oh, and #21471 (moved) will require a GET request for a v3 HS descriptor, and disabling the encrypted connection check in the function (or changing the mocked connection so it looks like it's encrypted).

dgoulet wrote a unit test for this, so it's not as essential.

Please see my branch test21470-029, which tests #21278 (moved), #21450 (moved), and #21507 (moved).

And we should add the http corpus file for #20894 (moved) at: https://github.com/teor2345/tor/blob/fuzz-dir-sensitive-v2/src/test/fuzz/fuzz_dir_testcase/dir-header-read-beyond-buffer.case

Once these are merged, we can close this ticket as done.

I want to defer fuzzing for issues with v3 hidden services (#21471 (moved)) - I've opened a ticket to implement v3 descriptor fuzzing: #21509 (moved). We can do this as part of testing hidden services in 0.3.0 or 0.3.1.

Trac:
Status: new to needs_review

I've merged test21470-029 to master, because #21507 (moved) isn't backported yet. Once it's backported, we can consider backporting this to 0.2.9 or 0.3.0.

I've added explicit regression-tests for #21278 (moved) and #20894 (moved) to our fuzzing corpora.

Moving this ticket to 0.3.0 for backport consideration.

Trac:
Keywords: test deleted, test 030-backport 029-backport added
Milestone: Tor: 0.3.1.x-final to Tor: 0.3.0.x-final

Trac:
Keywords: test 030-backport 029-backport deleted, 030-backport, 029-backport, test added

no backport.

Trac:
Milestone: Tor: 0.3.0.x-final to Tor: 0.3.1.x-final
Status: needs_review to closed
Resolution: N/A to fixed

closed

mentioned in issue #21507 (moved)

moved to tpo/core/tor#21470 (closed)