| Age | Commit message (Collapse) | Author |
|---|
|
Fixes #23910
Based on a patch by dgoulet; backported to 0.2.5
|
|
|
|
|
|
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 1c0e87f6d8c7a0abdadf1b5cd9082c10abc7f4e2
back in 0.2.4.11-alpha; it is bug 22737 and CID 1401591
|
|
|
|
On an hidden service rendezvous circuit, a BEGIN_DIR could be sent
(maliciously) which would trigger a tor_assert() because
connection_edge_process_relay_cell() thought that the circuit is an
or_circuit_t but is an origin circuit in reality.
Fixes #22494
Reported-by: Roger Dingledine <arma@torproject.org>
Signed-off-by: David Goulet <dgoulet@torproject.org>
|
|
|
|
|
|
|
|
Fixes bug 22490; bugfix on 6a241ff3ffe7dc1 in 0.2.4.6-alpha.
Found by teor using clang-5.0's AddressSanitizer stack-use-after-scope.
|
|
|
|
Fix for TROVE-2017-001 and bug 21278.
(Note: Instead of handling signed ints "correctly", we keep the old
behavior, except for the part where we would crash with -ftrapv.)
|
|
|
|
|
|
|
|
|
|
Add a changes file.
|
|
Fixes bug 16248. Patch from cypherpunks. Bugfix on 0.2.0.1-alpha.
|
|
|
|
This is a backport of 19728 and 19690
|
|
The length of auth_data from an INTRODUCE2 cell is checked when the
auth_type is recognized (1 or 2), but not for any other non-zero
auth_type. Later, auth_data is assumed to have at least
REND_DESC_COOKIE_LEN bytes, leading to a client-triggered out of bounds
read.
Fixed by checking auth_len before comparing the descriptor cookie
against known clients.
Fixes #15823; bugfix on 0.2.1.6-alpha.
|
|
This helps protect against bugs where any part of a buf_t's memory
is passed to a function that expects a NUL-terminated input.
|
|
This helps protect against bugs where any part of a buf_t's memory
is passed to a function that expects a NUL-terminated input.
|
|
|
|
|
|
In get_token(), we could read one byte past the end of the
region. This is only a big problem in the case where the region
itself is (a) potentially hostile, and (b) not explicitly
nul-terminated.
This patch fixes the underlying bug, and also makes sure that the
one remaining case of not-NUL-terminated potentially hostile data
gets NUL-terminated.
Fix for bug 21018, TROVE-2016-12-002, and CVE-2016-1254
|
|
|
|
It had been a directory authority since 0.2.1.20.
|
|
The problem is that "q" is always set on the first iteration even
if the question is not a supported question. This set of "q" is
not necessary, and will be handled after exiting the loop if there
if a supported q->type was found.
[Changes file by nickm]
lease enter the commit message for your changes. Lines starting
|
|
Conflicts:
src/or/config.c
|
|
This new identity key was changed on 18 November 2015.
|
|
|
|
|
|
|
|
There was a dead check when we made sure that an array member of a
struct was non-NULL. Tor has been doing this check since at least
0.2.3, maybe earlier.
Fixes bug 17781.
|
|
If crypto_early_init fails, a typo in a return value from tor_init
means that tor_main continues running, rather than returning
an error value.
Fixes bug 16360; bugfix on d3fb846d8c98 in 0.2.5.2-alpha,
introduced when implementing #4900.
Patch by "teor".
|
|
The length of auth_data from an INTRODUCE2 cell is checked when the
auth_type is recognized (1 or 2), but not for any other non-zero
auth_type. Later, auth_data is assumed to have at least
REND_DESC_COOKIE_LEN bytes, leading to a client-triggered out of bounds
read.
Fixed by checking auth_len before comparing the descriptor cookie
against known clients.
Fixes #15823; bugfix on 0.2.1.6-alpha.
|
|
|
|
In theory these should never the triggered as the only caller now
validates the parameters before this routine gets called.
|
|
Found by DonnchaC.
|
|
Fixes bug 15600; reported by skruffy
|
|
|
|
|
|
(Sending a nak would be pointless.)
See ticket 15515 for discussion.
|
|
|
|
This reverts commit 681802817deb6fb93b95f8284856fd42f3556600.
(I didn't mean to backport this, but somehow I had based my branch
for #15205 on it.)
|
|
|
|
|
|
|
|
Closes 14128; useful to regain functionality lost because of 13988.
|
