diff --git a/src/lib_ccx/file_buffer.h b/src/lib_ccx/file_buffer.h index 11973f5f1..577151c80 100644 --- a/src/lib_ccx/file_buffer.h +++ b/src/lib_ccx/file_buffer.h @@ -17,6 +17,20 @@ */ size_t buffered_read_opt(struct ccx_demuxer *ctx, unsigned char *buffer, size_t bytes); +/** + * Bytes still unread in the file buffer. + * + * bytesinbuffer and filebuffer_pos are both unsigned, so subtracting them without + * checking their order wraps around and turns a bounds check into a pass. Callers use + * this instead of doing the subtraction themselves. + */ +static inline size_t buffered_bytes_left(struct ccx_demuxer *ctx) +{ + if (ctx->filebuffer_pos >= ctx->bytesinbuffer) + return 0; + return (size_t)(ctx->bytesinbuffer - ctx->filebuffer_pos); +} + /** * Skip bytes from file buffer and if needed also seek file for number of bytes. * @@ -24,7 +38,7 @@ size_t buffered_read_opt(struct ccx_demuxer *ctx, unsigned char *buffer, size_t static size_t inline buffered_skip(struct ccx_demuxer *ctx, unsigned int bytes) { size_t result; - if (bytes <= ctx->bytesinbuffer - ctx->filebuffer_pos) + if (bytes <= buffered_bytes_left(ctx)) { ctx->filebuffer_pos += bytes; result = bytes; @@ -43,11 +57,14 @@ static size_t inline buffered_skip(struct ccx_demuxer *ctx, unsigned int bytes) static size_t inline buffered_read(struct ccx_demuxer *ctx, unsigned char *buffer, size_t bytes) { size_t result; - if (bytes <= ctx->bytesinbuffer - ctx->filebuffer_pos) + if (bytes <= buffered_bytes_left(ctx)) { if (buffer != NULL) memcpy(buffer, ctx->filebuffer + ctx->filebuffer_pos, bytes); - ctx->filebuffer_pos += bytes; + /* The guard above proved bytes fits in what is left of the buffer, and that + remainder is an unsigned int, so narrowing here cannot lose data. Say so + explicitly: MSVC warns C4267 on the implicit size_t -> unsigned int. */ + ctx->filebuffer_pos += (unsigned int)bytes; result = bytes; } else @@ -70,7 +87,7 @@ static size_t inline buffered_read(struct ccx_demuxer *ctx, unsigned char *buffe static size_t inline buffered_read_byte(struct ccx_demuxer *ctx, unsigned char *buffer) { size_t result = 0; - if (ctx->bytesinbuffer - ctx->filebuffer_pos) + if (buffered_bytes_left(ctx)) { if (buffer) { diff --git a/src/lib_ccx/file_functions.c b/src/lib_ccx/file_functions.c index fb8653b9f..194d4b946 100644 --- a/src/lib_ccx/file_functions.c +++ b/src/lib_ccx/file_functions.c @@ -230,17 +230,25 @@ void buffered_seek(struct ccx_demuxer *ctx, int offset) position_sanity_check(ctx); if (offset < 0) { - ctx->filebuffer_pos += offset; - if (ctx->filebuffer_pos < 0) + /* filebuffer_pos and startbytes_pos are unsigned, so "filebuffer_pos += offset" + wrapped around instead of going negative and neither guard below could ever + fire. A backward seek past the start of the buffer left filebuffer_pos at + roughly 2^32, which buffered_read() then trusted. Do the arithmetic signed. */ + int64_t newpos = (int64_t)ctx->filebuffer_pos + offset; + if (newpos < 0) { // We got into the start buffer (hopefully) - if ((ctx->filebuffer_pos + ctx->startbytes_pos) < 0) + if ((int64_t)ctx->startbytes_pos + newpos < 0) { fatal(CCX_COMMON_EXIT_BUG_BUG, "PANIC: Attempt to seek before buffer start, this is a bug!"); } - ctx->startbytes_pos += ctx->filebuffer_pos; + ctx->startbytes_pos = (unsigned int)((int64_t)ctx->startbytes_pos + newpos); ctx->filebuffer_pos = 0; } + else + { + ctx->filebuffer_pos = (unsigned int)newpos; + } } else {