mirror of
https://gitlab.torproject.org/tpo/core/tor.git
synced 2024-11-27 13:53:31 +01:00
mirror repository of the tor core protocol in case of issues
8d2978b13c
This mistake causes two possible bugs. I believe they are both
harmless IRL.
BUG 1: memory stomping
When we call the memset, we are overwriting two 0 bytes past the end
of packed_cell_t.body. But I think that's harmless in practice,
because the definition of packed_cell_t is:
// ...
typedef struct packed_cell_t {
TOR_SIMPLEQ_ENTRY(packed_cell_t) next;
char body[CELL_MAX_NETWORK_SIZE];
uint32_t inserted_time;
} packed_cell_t;
So we will overwrite either two bytes of inserted_time, or two bytes
of padding, depending on how the platform handles alignment.
If we're overwriting padding, that's safe.
If we are overwriting the inserted_time field, that's also safe: In
every case where we call cell_pack() from connection_or.c, we ignore
the inserted_time field. When we call cell_pack() from relay.c, we
don't set or use inserted_time until right after we have called
cell_pack(). SO I believe we're safe in that case too.
BUG 2: memory exposure
The original reason for this memset was to avoid the possibility of
accidentally leaking uninitialized ram to the network. Now
remember, if wide_circ_ids is false on a connection, we shouldn't
actually be sending more than 512 bytes of packed_cell_t.body, so
these two bytes can only leak to the network if there is another bug
somewhere else in the code that sends more data than is correct.
Fortunately, in relay.c, where we allocate packed_cell_t in
packed_cell_new() , we allocate it with tor_malloc_zero(), which
clears the RAM, right before we call cell_pack. So those
packed_cell_t.body bytes can't leak any information.
That leaves the two calls to cell_pack() in connection_or.c, which
use stack-alocated packed_cell_t instances.
In or_handshake_state_record_cell(), we pass the cell's contents to
crypto_digest_add_bytes(). When we do so, we get the number of
bytes to pass using the same setting of wide_circ_ids as we passed
to cell_pack(). So I believe that's safe.
In connection_or_write_cell_to_buf(), we also use the same setting
of wide_circ_ids in both calls. So I believe that's safe too.
I introduced this bug with
|
||
|---|---|---|
| changes | ||
| contrib | ||
| doc | ||
| m4 | ||
| src | ||
| .gitignore | ||
| acinclude.m4 | ||
| autogen.sh | ||
| ChangeLog | ||
| configure.ac | ||
| Doxyfile.in | ||
| INSTALL | ||
| LICENSE | ||
| Makefile.am | ||
| Makefile.nmake | ||
| README | ||
| ReleaseNotes | ||
Tor protects your privacy on the internet by hiding the connection
between your Internet address and the services you use. We believe Tor
is reasonably secure, but please ensure you read the instructions and
configure it properly.
To build Tor from source:
./configure && make && make install
Home page:
https://www.torproject.org/
Download new versions:
https://www.torproject.org/download.html
Documentation, including links to installation and setup instructions:
https://www.torproject.org/documentation.html
Making applications work with Tor:
https://wiki.torproject.org/noreply/TheOnionRouter/TorifyHOWTO
Frequently Asked Questions:
https://www.torproject.org/faq.html
https://wiki.torproject.org/noreply/TheOnionRouter/TorFAQ