148 lines
5.8 KiB
Diff
148 lines
5.8 KiB
Diff
From 210386037c892a720972ad35a3d8f7073b4d763b Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>
|
|
Date: Thu, 23 Apr 2026 18:04:24 +0200
|
|
Subject: [PATCH] Cope with integer overflow in data size arithmetics in
|
|
repo_add_solv()
|
|
|
|
When parsing solv files with maliciously large "maxsize" or "allsize"
|
|
data size, e.g. this maxsize value at offset 0x29--0x2E:
|
|
|
|
00000000 53 4f 4c 56 00 00 00 08 00 00 00 01 00 00 00 00 |SOLV............|
|
|
00000010 00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 01 |................|
|
|
00000020 00 00 00 00 00 00 00 00 00 8f ff ff bf 77 86 8d |.............w..|
|
|
00000030 20 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 | ...............|
|
|
00000040 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
|
|
*
|
|
00002030 00 |.|
|
|
00002031
|
|
|
|
read_id() function will decode and return 4294959095 value, and a subsequent
|
|
assignment:
|
|
|
|
int maxsize;
|
|
[...]
|
|
maxsize = read_id(&data, 0);
|
|
|
|
will experience an integer overflow on plaforms where signed int has 4-byte
|
|
size (e.g. x86_64).
|
|
|
|
The same flaw is possible at the next line:
|
|
|
|
allsize = read_id(&data, 0);
|
|
|
|
Subsequent arithmetics will interpreter the value as a very large negative
|
|
number, possibly doing wrong decisions:
|
|
|
|
maxsize += 5; /* so we can read the next schema of an array */
|
|
if (maxsize > allsize)
|
|
maxsize = allsize;
|
|
|
|
and finally, the negative value passed to solv_calloc():
|
|
|
|
buf = solv_calloc(maxsize + DATA_READ_CHUNK + 4, 1); /* 4 extra bytes to detect overflows */
|
|
|
|
will be coerced to an unsigned type (size_t) leading to allocating a smaller
|
|
buffer then intended. Then writing to the small buffer will experience a heap
|
|
buffer overflow:
|
|
|
|
l = maxsize;
|
|
if (l < DATA_READ_CHUNK)
|
|
l = DATA_READ_CHUNK;
|
|
if (l > allsize)
|
|
l = allsize;
|
|
if (!l || fread(buf, l, 1, data.fp) != 1)
|
|
|
|
This flaw can be demostrated by passing that solv file to the dumpsolv tool which
|
|
will crash if compiled with ASAN:
|
|
|
|
$ /tmp/b/tools/dumpsolv /tmp/vuln_1_101_1_negative_maxsize.solv
|
|
=================================================================
|
|
==17608==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7c0a2ede00b1 at pc 0x7fea30451468 b
|
|
p 0x7ffe07220a50 sp 0x7ffe07220220
|
|
WRITE of size 8192 at 0x7c0a2ede00b1 thread T0
|
|
#0 0x7fea30451467 in fread.part.0 (/lib64/libasan.so.8+0x51467) (BuildId: 80bfc4ae44fdec6ef5fecfb01
|
|
e2b57d28660991c)
|
|
#1 0x7fea3028eef1 in repo_add_solv /home/test/libsolv/src/repo_solv.c:1034
|
|
#2 0x0000004041cc in main /home/test/libsolv/tools/dumpsolv.c:471
|
|
#3 0x7fea3003c680 in __libc_start_call_main (/lib64/libc.so.6+0x3680) (BuildId: c04494d63bca865bedf571a4075ef8867ccf9fa9)
|
|
#4 0x7fea3003c797 in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x3797) (BuildId: c04494d63bca865bedf571a4075ef8867ccf9fa9)
|
|
#5 0x000000400694 in _start (/tmp/b/tools/dumpsolv+0x400694) (BuildId: 0a70b5b14e5cd81f90a309bb2ff3219dfbf30bb8)
|
|
|
|
0x7c0a2ede00b1 is located 0 bytes after 1-byte region [0x7c0a2ede00b0,0x7c0a2ede00b1)
|
|
allocated by thread T0 here:
|
|
#0 0x7fea304ef41f in malloc (/lib64/libasan.so.8+0xef41f) (BuildId: 80bfc4ae44fdec6ef5fecfb01e2b57d28660991c)
|
|
#1 0x7fea302e4b4c in solv_calloc /home/test/libsolv/src/util.c:77
|
|
#2 0x7fea3028ee38 in repo_add_solv /home/test/libsolv/src/repo_solv.c:1025
|
|
#3 0x0000004041cc in main /home/test/libsolv/tools/dumpsolv.c:471
|
|
#4 0x7fea3003c680 in __libc_start_call_main (/lib64/libc.so.6+0x3680) (BuildId: c04494d63bca865bedf571a4075ef8867ccf9fa9)
|
|
#5 0x7fea3003c797 in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x3797) (BuildId: c04494d63bca865bedf571a4075ef8867ccf9fa9)
|
|
#6 0x000000400694 in _start (/tmp/b/tools/dumpsolv+0x400694) (BuildId: 0a70b5b14e5cd81f90a309bb2ff3219dfbf30bb8)
|
|
|
|
SUMMARY: AddressSanitizer: heap-buffer-overflow /home/test/libsolv/src/repo_solv.c:1034 in repo_add_solv
|
|
|
|
This patch catches the integer overflow, sets an error and jumps to the end of
|
|
the function just after deallocation of the buffer (which would contain an
|
|
undefined pointer). This patch also handles a possible integer overflow at
|
|
"maxsize += 5" line.
|
|
|
|
I originally wanted to replace read_id() with read_u32(), but
|
|
complemtary repowriter_write() function also stored the value as
|
|
a signed integer, so I guess the the Id type is inteded there.
|
|
|
|
There are probably other ways how to fix it, like passing INT_MAX-5
|
|
limit to read_id(), though the error message would be less
|
|
understandable.
|
|
|
|
It's also possible to reject this patch with an explanation that loading
|
|
untrusted solv files is not supported. Though some kind of
|
|
fortification would be welcomed by people who debug solver problems
|
|
from reported solv files.
|
|
|
|
Reported by Aisle Research.
|
|
---
|
|
src/repo_solv.c | 14 ++++++++++++++
|
|
1 file changed, 14 insertions(+)
|
|
|
|
diff --git a/src/repo_solv.c b/src/repo_solv.c
|
|
index d98c973f..b8c981fa 100644
|
|
--- a/src/repo_solv.c
|
|
+++ b/src/repo_solv.c
|
|
@@ -18,6 +18,7 @@
|
|
#include <stdlib.h>
|
|
#include <unistd.h>
|
|
#include <string.h>
|
|
+#include <limits.h>
|
|
|
|
#include "repo_solv.h"
|
|
#include "util.h"
|
|
@@ -1018,6 +1019,18 @@ repo_add_solv(Repo *repo, FILE *fp, int flags)
|
|
|
|
maxsize = read_id(&data, 0);
|
|
allsize = read_id(&data, 0);
|
|
+ if (maxsize < 0 || allsize < 0)
|
|
+ {
|
|
+ data.error = pool_error(pool, SOLV_ERROR_CORRUPT, "negative data size in solv header");
|
|
+ id = 0;
|
|
+ goto data_error;
|
|
+ }
|
|
+ if (maxsize > INT_MAX - 5)
|
|
+ {
|
|
+ data.error = pool_error(pool, SOLV_ERROR_OVERFLOW, "data size overflow in solv header");
|
|
+ id = 0;
|
|
+ goto data_error;
|
|
+ }
|
|
maxsize += 5; /* so we can read the next schema of an array */
|
|
if (maxsize > allsize)
|
|
maxsize = allsize;
|
|
@@ -1343,6 +1356,7 @@ printf("=> %s %s %p\n", pool_id2str(pool, keys[key].name), pool_id2str(pool, key
|
|
}
|
|
solv_free(buf);
|
|
|
|
+data_error:
|
|
if (data.error)
|
|
{
|
|
/* free solvables */
|
|
--
|
|
2.54.0
|
|
|