Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#4410 closed defect (fixed)

Assertion failure in client in rend_decrypt_introduction_points()

Reported by: drosenbe Owned by:
Priority: High Milestone:
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Keywords: tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

It appears to be possible to remotely trigger an assertion failure in the client when it's parsing directory authority responses.

If a client receives a response with purpose DIR_PURPOSE_FETCH_RENDDESC_V2, it calls rend_cache_store_v2_desc_as_client() (in or/rendcommon.c), which immediately calls rend_parse_v2_service_descriptor() (in or/routerparse.c) to parse the descriptor. This will parse the descriptor and pull out the encrypted introduction points without validation on contents or size (to be performed later).

Then, back in rend_cache_store_v2_desc_as_client(), rend_decrypt_introduction_points() (in or/routerparse.c) is called. This function verifies the size of the encrypted introduction isn't less than 2. If the type is REND_STEALTH_AUTH, the following is immediately called:

dec = tor_malloc_zero(ipos_encrypted_size - CIPHER_IV_LEN - 1);

If the size is less than CIPHER_IV_LEN, this will underflow and the allocation attempt on a huge size will trigger an assertion failure.

Child Tickets

Attachments (1)

decrypt_intro.patch (748 bytes) - added by drosenbe 8 years ago.
Patch for assertion failure in encrypted introduction parsing

Download all attachments as: .zip

Change History (9)

Changed 8 years ago by drosenbe

Attachment: decrypt_intro.patch added

Patch for assertion failure in encrypted introduction parsing

comment:1 Changed 8 years ago by drosenbe

Summary: Remote assertion failure in clientAssertion failure in client in rend_decrypt_introduction_points()

comment:2 Changed 8 years ago by Sebastian

Milestone: Tor: 0.2.1.x-final
Status: newneeds_review

This goes all the way back to 0.2.1.

comment:3 Changed 8 years ago by Sebastian

Priority: normalmajor

branch bug4410 in my repo. added a changes file, made up a commit message. Please sanity-check. Should we demote/reword all those log_warn()'s that won't tell the user anything about what's up?

comment:4 in reply to:  3 Changed 8 years ago by rransom

Replying to Sebastian:

branch bug4410 in my repo. added a changes file, made up a commit message. Please sanity-check.

Looks good!

Should we demote/reword all those log_warn()'s that won't tell the user anything about what's up?

Messages about failures in rend_decrypt_introduction_points need to be warning messages, because they give useful information about why a hidden service ‘doesn't work’. Warnings in rend_cache_store_v2_desc_as_client should give useful information about which HS descriptor is being parsed, but that's a different bug.

comment:5 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Looks correct to me; merging. Thanks, all!

comment:6 Changed 7 years ago by nickm

Keywords: tor-client added

comment:7 Changed 7 years ago by nickm

Component: Tor ClientTor

comment:8 Changed 7 years ago by nickm

Milestone: Tor: 0.2.1.x-final

Milestone Tor: 0.2.1.x-final deleted

Note: See TracTickets for help on using tickets.