diff options
author | Patrick Steinhardt <ps@pks.im> | 2019-01-23 14:45:19 +0100 |
---|---|---|
committer | Edward Thomson <ethomson@edwardthomson.com> | 2019-01-31 13:53:28 +0000 |
commit | 0ceac0d0ce592c8c26575a12d47f5371e69aff79 (patch) | |
tree | ac43248aaeeed91ed5f30ad55de65262bce59803 | |
parent | 75918aba1d0e4a730d29ac7ea737cc3d234cf333 (diff) | |
download | libgit2-0ceac0d0ce592c8c26575a12d47f5371e69aff79.tar.gz |
mbedtls: fix potential size overflow when reading or writing dataethomson/stream-truncated-writes
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.
-rw-r--r-- | src/streams/mbedtls.c | 11 |
1 files changed, 9 insertions, 2 deletions
diff --git a/src/streams/mbedtls.c b/src/streams/mbedtls.c index 2ade83ce1..cbe2f681a 100644 --- a/src/streams/mbedtls.c +++ b/src/streams/mbedtls.c @@ -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); |