105 lines
4.2 KiB
Diff
105 lines
4.2 KiB
Diff
|
From 8b40e8021a98728b5889516af308dd52378c964c Mon Sep 17 00:00:00 2001
|
|||
|
|
From: Lv Ying <lvying6@huawei.com>
|
|||
|
|
Date: Wed, 23 Feb 2022 09:32:21 +0800
|
|||
|
|
Subject: [PATCH 2/2] fix CVE-2022-0529 Heap out-of-bound writes and reads
|
|||
|
|
during conversion of wide string to local string
|
|||
|
|
MIME-Version: 1.0
|
|||
|
|
Content-Type: text/plain; charset=UTF-8
|
|||
|
|
Content-Transfer-Encoding: 8bit
|
|||
|
|
|
|||
|
|
CVE-2022-0529 discussed in https://bugzilla.redhat.com/show_bug.cgi?id=2051402
|
|||
|
|
CVE can be reproduced:
|
|||
|
|
$ unset LANG
|
|||
|
|
$ valgrind ./unzip ./unzip_03/testcase
|
|||
|
|
valgrind will detect Heap out-of-bound writes and reads just as bugzilla discussed
|
|||
|
|
|
|||
|
|
This is because wide_to_escape_string returns a string that represents a wide char
|
|||
|
|
not in local char set, is longer than MAX_ESCAPE_BYTES(8). Actually, MAX_ESCAPE_BYTES
|
|||
|
|
max is 10, for example, 4-byte wide character '#L02020276' is 10 bytes long, not
|
|||
|
|
including the terminating null character. So strcat(buffer, escape_string) will cause
|
|||
|
|
Heap out-of-bound writes.
|
|||
|
|
|
|||
|
|
By default, the OS vendor sets the LANG environment variable. valgrind tests this POC
|
|||
|
|
will get another memory error.
|
|||
|
|
$ export | grep LANG
|
|||
|
|
declare -x LANG="en_US.UTF-8"
|
|||
|
|
$ valgrind ./unzip ./unzip_03/testcase
|
|||
|
|
|
|||
|
|
Archive: unzip_03/testcase
|
|||
|
|
warning [unzip_03/testcase]: 303 extra bytes at beginning or within zipfile
|
|||
|
|
(attempting to process anyway)
|
|||
|
|
error [unzip_03/testcase]: reported length of central directory is
|
|||
|
|
-303 bytes too long (Atari STZip zipfile? J.H.Holm ZIPSPLIT 1.1
|
|||
|
|
zipfile?). Compensating...
|
|||
|
|
==15725== Conditional jump or move depends on uninitialised value(s)
|
|||
|
|
==15725== at 0x4903169: __wcsnlen_sse4_1 (strlen.S:186)
|
|||
|
|
==15725== by 0x48F3D61: wcsrtombs (wcsrtombs.c:104)
|
|||
|
|
==15725== by 0x488B9A0: wcstombs (wcstombs.c:34)
|
|||
|
|
==15725== by 0x407279: wcstombs (stdlib.h:154)
|
|||
|
|
==15725== by 0x407279: fnfilter.constprop.2 (extract.c:2946)
|
|||
|
|
==15725== by 0x4076A5: store_info (extract.c:1155)
|
|||
|
|
==15725== by 0x40AFF4: extract_or_test_files (extract.c:782)
|
|||
|
|
==15725== by 0x41586C: do_seekable (process.c:994)
|
|||
|
|
==15725== by 0x4167EE: process_zipfiles (process.c:401)
|
|||
|
|
==15725== by 0x40449B: unzip (unzip.c:1280)
|
|||
|
|
==15725== by 0x4874B26: (below main) (libc-start.c:308)
|
|||
|
|
==15725==
|
|||
|
|
skipping: ??????????????????????????????????????????????????????????????????????????ı need PK compat. v4.6 (can do v4.5)
|
|||
|
|
==15725==
|
|||
|
|
==15725== HEAP SUMMARY:
|
|||
|
|
==15725== in use at exit: 0 bytes in 0 blocks
|
|||
|
|
==15725== total heap usage: 37 allocs, 37 frees, 90,739 bytes allocated
|
|||
|
|
==15725==
|
|||
|
|
==15725== All heap blocks were freed -- no leaks are possible
|
|||
|
|
==15725==
|
|||
|
|
==15725== For counts of detected and suppressed errors, rerun with: -v
|
|||
|
|
==15725== Use --track-origins=yes to see where uninitialised values come from
|
|||
|
|
==15725== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
|
|||
|
|
|
|||
|
|
This is because wcstombs( newraw, wostring, (woslen * MB_CUR_MAX) + 1) in fnfilter
|
|||
|
|
use wrong n parameter which stands for At most n bytes are written to dest.
|
|||
|
|
When LANG environment variable is set, MB_CUR_MAX = 6, so wcstombs will writes more
|
|||
|
|
bytes over dest(newraw).
|
|||
|
|
|
|||
|
|
Signed-off-by: Lv Ying <lvying6@huawei.com>
|
|||
|
|
---
|
|||
|
|
extract.c | 4 ++--
|
|||
|
|
process.c | 2 +-
|
|||
|
|
2 files changed, 3 insertions(+), 3 deletions(-)
|
|||
|
|
|
|||
|
|
diff --git a/extract.c b/extract.c
|
|||
|
|
index f0e8217..3f6e14d 100644
|
|||
|
|
--- a/extract.c
|
|||
|
|
+++ b/extract.c
|
|||
|
|
@@ -2856,12 +2856,12 @@ char *fnfilter(raw, space, size) /* convert name to safely printable form */
|
|||
|
|
if (wslen != (size_t)-1)
|
|||
|
|
{
|
|||
|
|
/* Apparently valid Unicode. Allocate wide-char storage. */
|
|||
|
|
- wstring = (wchar_t *)malloc((wslen + 1) * sizeof(wchar_t));
|
|||
|
|
+ wstring = (wchar_t *)calloc((wslen + 1), sizeof(wchar_t));
|
|||
|
|
if (wstring == NULL) {
|
|||
|
|
strcpy( (char *)space, raw);
|
|||
|
|
return (char *)space;
|
|||
|
|
}
|
|||
|
|
- wostring = (wchar_t *)malloc(2 * (wslen + 1) * sizeof(wchar_t));
|
|||
|
|
+ wostring = (wchar_t *)calloc(2 * (wslen + 1), sizeof(wchar_t));
|
|||
|
|
if (wostring == NULL) {
|
|||
|
|
free(wstring);
|
|||
|
|
strcpy( (char *)space, raw);
|
|||
|
|
diff --git a/process.c b/process.c
|
|||
|
|
index 5cba073..3e7fcb3 100644
|
|||
|
|
--- a/process.c
|
|||
|
|
+++ b/process.c
|
|||
|
|
@@ -2395,7 +2395,7 @@ char *local_to_utf8_string(local_string)
|
|||
|
|
*/
|
|||
|
|
|
|||
|
|
/* set this to the max bytes an escape can be */
|
|||
|
|
-#define MAX_ESCAPE_BYTES 8
|
|||
|
|
+#define MAX_ESCAPE_BYTES 10
|
|||
|
|
|
|||
|
|
char *wide_to_escape_string(wide_char)
|
|||
|
|
zwchar wide_char;
|
|||
|
|
--
|
|||
|
|
2.27.0
|
|||
|
|
|