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

Jump to bottom

#33788: Make things safer when tor_inet_ntop() and tor_inet_ntoa() fail (2) #1902

Merged
merged 9 commits into from Jun 5, 2020
Merged

#33788: Make things safer when tor_inet_ntop() and tor_inet_ntoa() fail (2) #1902

merged 9 commits into from Jun 5, 2020

Conversation

Reviewers

@nmathewson nmathewson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@rl1987 @coveralls @nmathewson @torproject-pusher
@rl1987
Copy link
Contributor

@rl1987 rl1987 commented May 21, 2020

https://trac.torproject.org/projects/tor/ticket/33788
rl1987 and others added 8 commits May 21, 2020
@rl1987
Check output value of tor_inet_ntop/tor_inet_ntoa in dns.c
b5bfdbf
@rl1987
Check for possible failures of tor_inet_ntop/tor_inet_ntoa in fmt_add…
d8e2468 
…r32 and tor_dup_ip
@rl1987
Check that fmt_addr32() returned non-empty string before putting it i…
8ab7e30 
…nto documents
@rl1987
Add changes file
17c4e48
@rl1987
Actually, bail out of routerstatus_format_entry() if fmt_addr32() fails
38cbfda
@rl1987
Bail out of format_networkstatus_vote if fmt_addr32() failed.
ded9999 
Something is fishy if we cannot put IP address string into dir-vote line.
@rl1987
Check for NULL from tor_dup_ip()
a5d28bf
@rl1987
Tweak format string
7a004fc
@coveralls
Copy link

@coveralls coveralls commented May 21, 2020 •
edited

Pull Request Test Coverage Report for Build 9048

  • 16 of 58 (27.59%) changed or added relevant lines in 13 files are covered.
  • 323 unchanged lines in 8 files lost coverage.
  • Overall coverage decreased (-0.02%) to 64.181%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/feature/control/control_getinfo.c 0 1 0.0%
src/feature/nodelist/dirlist.c 0 1 0.0%
src/core/or/connection_edge.c 0 2 0.0%
src/feature/client/circpathbias.c 0 3 0.0%
src/feature/dirauth/dirvote.c 6 9 66.67%
src/feature/nodelist/authcert.c 0 3 0.0%
src/feature/relay/selftest.c 0 3 0.0%
src/feature/rend/rendservice.c 0 3 0.0%
src/feature/relay/relay_periodic.c 0 10 0.0%
src/feature/relay/dns.c 0 13 0.0%
Files with Coverage Reduction New Missed Lines %
src/feature/relay/dns.c 2 24.7%
src/feature/rend/rendservice.c 2 32.37%
src/feature/relay/selftest.c 3 0%
src/lib/net/address.c 7 90.91%
src/feature/hs/hs_service.c 24 74.6%
src/feature/client/entrynodes.c 39 86.21%
src/feature/control/control_getinfo.c 54 28.63%
src/app/main/main.c 192 22.0%
Totals Coverage Status
Change from base Build 8974: -0.02%
Covered Lines: 50961
Relevant Lines: 79402
💛 - Coveralls

nmathewson
nmathewson reviewed Jun 1, 2020
View changes
src/feature/relay/dns.c Outdated
tor_inet_ntoa(&in, answer_buf, sizeof(answer_buf));
wildcard_increment_answer(answer_buf);
ntoa_res = tor_inet_ntoa(&in, answer_buf, sizeof(answer_buf));
tor_assert_nonfatal(ntoa_res > 0);
Copy link
Contributor

@nmathewson nmathewson Jun 1, 2020

This is incorrect; look at the documentation for tor_inet_ntoa(). It says "returns -1 on failure" and "returns non-negative integer on success." So the test should be >= 0.

Copy link
Contributor Author

@rl1987 rl1987 Jun 5, 2020 •
edited

All three checks for tor_inet_ntoa() return value fixed.

src/lib/net/address.c Outdated Show resolved Hide resolved
nmathewson
nmathewson reviewed Jun 1, 2020
View changes
src/lib/net/address.c Outdated Show resolved Hide resolved
@rl1987
Fix some checks of tor_inet_ntoa() return value
3e4814e 
Also, fix a format string.
@torproject-pusher torproject-pusher merged commit 1fb9be5 into torproject:master Jun 5, 2020
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment