[FIX] Out-of-bounds read: unsigned underflow defeats the file buffer bounds checks

src/lib_ccx/file_buffer.h

@@ -17,14 +17,28 @@
  */
 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.
  *
  */
 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)
 		{

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
 	{