tls-socket: Avoid accessing stale data when processing application data
In non-blocking mode, the previous code set `in_done` to -1 (SIZE_MAX) if `recv()` would block and nothing was read yet. If this was followed by a call to `write()` and `process()` is called and actually processed application data, the length calculation in the callback underflows and `memcpy()` would write to `in.ptr + SIZE_MAX`. Since `read()` already sets `errno` to `EWOULDBLOCK` and returns -1 if `in_done` is 0, the removed check was redundant anyway. Also, the buffer from the previous `read()` call might not be valid anymore when `write()` is called (e.g. `splice()` uses the same buffer for both, and the buffer could even be defined on a now invalid stack frame of the function that called `read()` previously). Clearing the data avoids that and ensures the application data is cached until the next call to `read()`. However, triggering this is rather difficult as `write()` should only reach `recv()` while the handshake isn't complete and until then `process_application()` doesn't accept application data. But if the handshake is completed during a call to `write()` that follows a non-blocking read and data immediately arrives, it's theoretically imaginable. This scenario is highly unlikely on a TLS server, which starts the process with a call to `read()` that then basically loops until the handshake is done. Even if multiple calls are required, the server will generally not call `write()` before it received application data. And any calls to `write()` afterwards do not reach `recv()` anymore (unless no data to send was passed, which would be weird, or maybe for some weird corner case that lets `build()` fail before all outbound application data was processed).
This commit is contained in:
+5
-11
@@ -230,17 +230,7 @@ static bool exchange(private_tls_socket_t *this, bool wr, bool block)
|
||||
in = recv(this->fd, buf, sizeof(buf), flags);
|
||||
if (in < 0)
|
||||
{
|
||||
if (errno == EAGAIN || errno == EWOULDBLOCK)
|
||||
{
|
||||
if (this->app.in_done == 0)
|
||||
{
|
||||
/* reading, nothing got yet, and call would block */
|
||||
errno = EWOULDBLOCK;
|
||||
this->app.in_done = -1;
|
||||
}
|
||||
return TRUE;
|
||||
}
|
||||
return FALSE;
|
||||
return errno == EAGAIN || errno == EWOULDBLOCK;
|
||||
}
|
||||
if (in == 0)
|
||||
{ /* EOF */
|
||||
@@ -299,6 +289,10 @@ METHOD(tls_socket_t, read_, ssize_t,
|
||||
METHOD(tls_socket_t, write_, ssize_t,
|
||||
private_tls_socket_t *this, void *buf, size_t len)
|
||||
{
|
||||
/* clear data from a previous read() call */
|
||||
this->app.in = chunk_empty;
|
||||
this->app.in_done = 0;
|
||||
|
||||
this->app.out.ptr = buf;
|
||||
this->app.out.len = len;
|
||||
this->app.out_done = 0;
|
||||
|
||||
Reference in New Issue
Block a user