Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug28223 diagnostic 035 #891

Merged

Conversation

Labels
None yet
Projects
None yet
4 participants
@nmathewson
Copy link
Contributor

@nmathewson nmathewson commented Apr 3, 2019

No description provided.

nmathewson added 2 commits Apr 3, 2019
Try to figure out _where exactly_ we are first encountering NULs in
microdescriptors, and what we are doing when that happens.
This is just in case there is some rogue platform that uses a
nonstandard value for SEEK_*, and does not define that macro in
unistd.h.  I think that's unlikely, but it's conceivable.
@coveralls
Copy link

@coveralls coveralls commented Apr 3, 2019

Pull Request Test Coverage Report for Build 4741

  • 11 of 22 (50.0%) changed or added relevant lines in 2 files are covered.
  • 1226 unchanged lines in 9 files lost coverage.
  • Overall coverage decreased (-0.005%) to 61.709%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/feature/dirparse/microdesc_parse.c 3 7 42.86%
src/feature/nodelist/microdesc.c 8 15 53.33%
Files with Coverage Reduction New Missed Lines %
src/ext/getdelim.c 1 0.0%
src/app/main/main.c 1 27.83%
src/feature/hs/hs_common.c 1 83.28%
src/lib/buf/buffers.c 6 91.03%
src/core/or/circuitpadding.c 12 93.61%
src/feature/nodelist/networkstatus.c 137 53.29%
src/core/mainloop/connection.c 233 25.58%
src/feature/dirauth/dirvote.c 372 64.74%
src/app/config/config.c 463 72.5%
Totals Coverage Status
Change from base Build 4539: -0.005%
Covered Lines: 45349
Relevant Lines: 73489

💛 - Coveralls

log_warn(LD_BUG, "About to dump a NUL into a microdescriptor file. "
"offset %"PRId64", bodylen %zu, nul position %zu",
(int64_t)md->off, md->bodylen,
(size_t)(nulpos - md->body));
Copy link
Contributor

@teor2345 teor2345 Apr 15, 2019

Can we log the escaped() context of the NUL?
Maybe 10 bytes either side of the NUL?

Copy link
Contributor

@teor2345 teor2345 Apr 15, 2019

Also, can we log some identifier from the microdescriptor?

Copy link
Contributor

@teor2345 teor2345 Apr 15, 2019

Should we call warn_if_nul_found() here?

Copy link
Contributor Author

@nmathewson nmathewson Apr 15, 2019

I'll try to do the other stuff; at this stage, there is no identifier from the md that we could log.

const char *nulpos = memchr(md->body, 0, md->bodylen);
if (BUG(nulpos)) {
log_warn(LD_BUG, "About to dump a NUL into a microdescriptor file. "
"offset %"PRId64", bodylen %zu, nul position %zu",
Copy link
Contributor

@teor2345 teor2345 Apr 15, 2019

Windows says:
"error: unknown conversion type character 'z' in format"
https://ci.appveyor.com/project/torproject/tor/builds/23576412/job/80p2k2cumplatqx9#L1468

const char *nul_found = memchr(inp, 0, len);
if (BUG(nul_found)) {
log_warn(LD_BUG, "Found unexpected NUL while reading %s, at "
"position %zu/%zu.",
Copy link
Contributor

@teor2345 teor2345 Apr 15, 2019

Windows probably doesn't like this z, either.

nmathewson added 2 commits Apr 15, 2019
We need to encode here instead of doing escaped(), since fwict
escaped() does not currently handle NUL bytes.

Also, use warn_if_nul_found in more cases to avoid duplication.
@torproject-pusher torproject-pusher merged commit 950d890 into torproject:maint-0.4.0 Apr 18, 2019
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment