hs: Fix bad use of sizeof() when encoding ESTABLISH_INTRO legacy cell

When encoding a legacy ESTABLISH_INTRO cell, we were using the sizeof() on a
pointer instead of using the real size of the destination buffer leading to an
overflow passing an enormous value to the signing digest function.
Fortunately, that value was only used to make sure the destination buffer
length was big enough for the key size and in this case it always was because
of the overflow.

Fixes #21553

Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit is contained in:
David Goulet 2017-02-24 09:48:14 -05:00 committed by Nick Mathewson
parent 823fb68a14
commit 4ed10e5053
4 changed files with 16 additions and 5 deletions

7
changes/bug21553 Normal file
View File

@ -0,0 +1,7 @@
o Minor bugfixes (hidden service):
- When encoding a legacy ESTABLISH_INTRO cell, we were using the sizeof()
on a pointer instead of real size of the destination buffer leading to
an overflow passing an enormous value to the signing digest function.
Fortunately, that value was only used to make sure the destination
buffer length was big enough for the key size and in this case it was.
Fixes bug 21553; bugfix on tor-0.3.0.1-alpha.

View File

@ -3174,8 +3174,9 @@ count_intro_point_circuits(const rend_service_t *service)
of bytes written. On fail, return -1.
*/
STATIC ssize_t
encode_establish_intro_cell_legacy(char *cell_body_out, crypto_pk_t *intro_key,
char *rend_circ_nonce)
encode_establish_intro_cell_legacy(char *cell_body_out,
size_t cell_body_out_len,
crypto_pk_t *intro_key, char *rend_circ_nonce)
{
int retval = -1;
int r;
@ -3202,7 +3203,7 @@ encode_establish_intro_cell_legacy(char *cell_body_out, crypto_pk_t *intro_key,
len += 20;
note_crypto_pk_op(REND_SERVER);
r = crypto_pk_private_sign_digest(intro_key, cell_body_out+len,
sizeof(cell_body_out)-len,
cell_body_out_len - len,
cell_body_out, len);
if (r<0) {
log_warn(LD_BUG, "Internal error: couldn't sign introduction request.");
@ -3313,8 +3314,9 @@ rend_service_intro_has_opened(origin_circuit_t *circuit)
/* Send the ESTABLISH_INTRO cell */
{
ssize_t len;
len = encode_establish_intro_cell_legacy(buf, circuit->intro_key,
circuit->cpath->prev->rend_circ_nonce);
len = encode_establish_intro_cell_legacy(buf, sizeof(buf),
circuit->intro_key,
circuit->cpath->prev->rend_circ_nonce);
if (len < 0) {
reason = END_CIRC_REASON_INTERNAL;
goto err;

View File

@ -130,6 +130,7 @@ STATIC int rend_service_poison_new_single_onion_dir(
const rend_service_t *s,
const or_options_t* options);
STATIC ssize_t encode_establish_intro_cell_legacy(char *cell_body_out,
size_t cell_body_out_len,
crypto_pk_t *intro_key,
char *rend_circ_nonce);
STATIC void prune_services_on_reload(smartlist_t *old_service_list,

View File

@ -489,6 +489,7 @@ helper_establish_intro_v2(or_circuit_t *intro_circ)
/* Use old circuit_key_material why not */
cell_len = encode_establish_intro_cell_legacy((char*)cell_body,
sizeof(cell_body),
key1,
(char *) circuit_key_material);
tt_int_op(cell_len, >, 0);