Opened 7 days ago

Last modified 6 days ago

#32673 assigned defect

'buf_read_from_tls()' can return the wrong error code

Reported by: opara Owned by: nickm
Priority: Medium Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor Version: Tor: 0.3.0.4-rc
Severity: Normal Keywords: tor-tls, tor-security, 035-backport, 040-backport, 041-backport, 042-backport
Cc: Actual Points:
Parent ID: Points: 1
Reviewer: Sponsor:

Description

The function buf_read_from_tls(...) returns an integer. This integer can either be <=0 (in which case it corresponds to a TOR_TLS_ status) or a positive number (in which case it corresponds to the number of bytes read). This return value is used in connection_buf_read_from_socket() in a large switch(result) statement.

At the beginning of buf_read_from_tls(...), it returns -1 on the lines:

IF_BUG_ONCE(buf->datalen >= INT_MAX)
  return -1;
IF_BUG_ONCE(buf->datalen >= INT_MAX - at_most)
  return -1;

This value of -1 is the same as TOR_TLS_WANTWRITE. This causes the switch statement in connection_buf_read_from_socket() to interpret the return value as TOR_TLS_WANTWRITE, which is not correct for the buf->datalen >= INT_MAX bug. I suggest returning TOR_TLS_ERROR_MISC instead of -1. Note that this would close the connection.

I don't think you'll see incorrect behavior due to this, but it might be a good idea to fix.

Child Tickets

Change History (3)

comment:1 Changed 7 days ago by teor

Keywords: tor-tls tor-security added
Milestone: Tor: 0.4.3.x-final
Owner: set to nickm
Points: 1
Status: newassigned

I think Nick is the best person to triage this bug, and decide if it should block our planned releases in the next 2 weeks.

comment:2 Changed 7 days ago by opara

For reference, this code was added in commit ee5471f9 in response to #21369.

comment:3 Changed 6 days ago by teor

Keywords: 035-backport 040-backport 041-backport 042-backport added
Version: Tor: 0.3.0.4-rc
Note: See TracTickets for help on using tickets.