[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #17776 [Tor]: Buffer over-reads in directory and rendcache tests
#17776: Buffer over-reads in directory and rendcache tests
-------------------------------+------------------------------------
Reporter: cypherpunks | Owner: cypherpunks
Type: defect | Status: assigned
Priority: Medium | Milestone: Tor: 0.2.8.x-final
Component: Tor | Version: Tor: 0.2.7.3-rc
Severity: Normal | Resolution:
Keywords: TorCoreTeam201512 | Actual Points:
Parent ID: | Points:
Sponsor: |
-------------------------------+------------------------------------
Comment (by teor):
Replying to [comment:8 cypherpunks]:
> Replying to [comment:7 teor]:
> > Replying to [comment:6 cypherpunks]:
> > > Replying to [comment:5 teor]:
> > > > Replying to [comment:4 cypherpunks]:
> > > > > Replying to [comment:2 teor]:
> > > > > > I want to check that the fingerprint strings are:
> > > > > > * not NULL, and
> > > > > > * don't contain a NULL character in the first DIGEST_LEN
bytes?
> > > > > > before the functions read the strings?
> > > > > >
> > > > > > You can use code like:
> > > > > > {{{
> > > > > > tor_assert(fingerprint);
> > > > > > tor_assert(memchr(fingerprint, 0, DIGEST_LEN) == NULL);
> > > > > > }}}
> > > > > >
> > > > > > That would also require updating all the test data so it's
really DIGEST_LEN characters long (and increasing the buffer lengths by 1
to accommodate the terminating nul byte).
> > > > >
> > > > > The bugs were found using AddressSanitizer and i think the
`memchr` solution will trigger similar warnings because the behavior is
undefined if access occurs beyond the end of the array searched.
> > > >
> > > > memchr(fingerprint, 0, DIGEST_LEN) will never access any byte in
fingerprint beyond a nul byte or DIGEST_LEN bytes. (Whichever comes
first.)
> > > Unless the fingerprint isn't null-terminated and adjacent memory
happens to contain random data with a null-byte exactly at DIGEST_LEN+1.
This would both be undefined behavior because it over-reads and it would
see the fingerprint as being of valid length which it isn't.
> >
> > This isn't how memchr works. It only ever reads "n" bytes, where "n"
is its third argument. If we pass DIGEST_LEN for n, it will never read
byte DIGEST_LEN + 1 under any circumstances. (But see my comments below
for why this doesn't matter.)
> I believe my argument is still valid even when "n" is DIGEST_LEN (i did
make a off-by-one error in my initial argument). Imagine if fingerprint
has a length of 1 but that single character isn't a null-byte, `memchr`
will then over-read until it finds a null-byte or has read DIGEST_LEN - 1
bytes of adjacent memory. I'm not trying to beat a dead horse or be
defensive, just trying to explain the issue with `memchr`.
I think we agree about how memchr works, and that it isn't a perfect
solution.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/17776#comment:9>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs