Commit 0ceac0d0 by Patrick Steinhardt Committed by Edward Thomson

mbedtls: fix potential size overflow when reading or writing data

The mbedtls library uses a callback mechanism to allow downstream users
to plug in their own receive and send functions. We implement `bio_read`
and `bio_write` functions, which simply wrap the `git_stream_read` and
`git_stream_write` functions, respectively.

The problem arises due to the return value of the callback functions:
mbedtls expects us to return an `int` containing the actual number of
bytes that were read or written. But this is in fact completely
misdesigned, as callers are allowed to pass in a buffer with length
`SIZE_MAX`. We thus may be unable to represent the number of bytes
written via the return value.

Fix this by only ever reading or writing at most `INT_MAX` bytes.
parent 75918aba
......@@ -169,13 +169,13 @@ cleanup:
static int bio_read(void *b, unsigned char *buf, size_t len)
{
git_stream *io = (git_stream *) b;
return (int) git_stream_read(io, buf, len);
return (int) git_stream_read(io, buf, min(len, INT_MAX));
}
static int bio_write(void *b, const unsigned char *buf, size_t len)
{
git_stream *io = (git_stream *) b;
return (int) git_stream_write(io, (const char *)buf, len, 0);
return (int) git_stream_write(io, (const char *)buf, min(len, INT_MAX), 0);
}
static int ssl_set_error(mbedtls_ssl_context *ssl, int error)
......@@ -308,6 +308,13 @@ static ssize_t mbedtls_stream_write(git_stream *stream, const char *data, size_t
GIT_UNUSED(flags);
/*
* `mbedtls_ssl_write` can only represent INT_MAX bytes
* written via its return value. We thus need to clamp
* the maximum number of bytes written.
*/
len = min(len, INT_MAX);
if ((written = mbedtls_ssl_write(st->ssl, (const unsigned char *)data, len)) <= 0)
return ssl_set_error(st->ssl, written);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment