Brad Bishop | 1932369 | 2019-04-05 15:28:33 -0400 | [diff] [blame^] | 1 | CVE: CVE-2018-1000878 |
| 2 | Upstream-Status: Backport |
| 3 | Signed-off-by: Ross Burton <ross.burton@intel.com> |
| 4 | |
| 5 | From bfcfe6f04ed20db2504db8a254d1f40a1d84eb28 Mon Sep 17 00:00:00 2001 |
| 6 | From: Daniel Axtens <dja@axtens.net> |
| 7 | Date: Tue, 4 Dec 2018 00:55:22 +1100 |
| 8 | Subject: [PATCH] rar: file split across multi-part archives must match |
| 9 | |
| 10 | Fuzzing uncovered some UAF and memory overrun bugs where a file in a |
| 11 | single file archive reported that it was split across multiple |
| 12 | volumes. This was caused by ppmd7 operations calling |
| 13 | rar_br_fillup. This would invoke rar_read_ahead, which would in some |
| 14 | situations invoke archive_read_format_rar_read_header. That would |
| 15 | check the new file name against the old file name, and if they didn't |
| 16 | match up it would free the ppmd7 buffer and allocate a new |
| 17 | one. However, because the ppmd7 decoder wasn't actually done with the |
| 18 | buffer, it would continue to used the freed buffer. Both reads and |
| 19 | writes to the freed region can be observed. |
| 20 | |
| 21 | This is quite tricky to solve: once the buffer has been freed it is |
| 22 | too late, as the ppmd7 decoder functions almost universally assume |
| 23 | success - there's no way for ppmd_read to signal error, nor are there |
| 24 | good ways for functions like Range_Normalise to propagate them. So we |
| 25 | can't detect after the fact that we're in an invalid state - e.g. by |
| 26 | checking rar->cursor, we have to prevent ourselves from ever ending up |
| 27 | there. So, when we are in the dangerous part or rar_read_ahead that |
| 28 | assumes a valid split, we set a flag force read_header to either go |
| 29 | down the path for split files or bail. This means that the ppmd7 |
| 30 | decoder keeps a valid buffer and just runs out of data. |
| 31 | |
| 32 | Found with a combination of AFL, afl-rb and qsym. |
| 33 | --- |
| 34 | libarchive/archive_read_support_format_rar.c | 9 +++++++++ |
| 35 | 1 file changed, 9 insertions(+) |
| 36 | |
| 37 | diff --git a/libarchive/archive_read_support_format_rar.c b/libarchive/archive_read_support_format_rar.c |
| 38 | index 6f419c27..a8cc5c94 100644 |
| 39 | --- a/libarchive/archive_read_support_format_rar.c |
| 40 | +++ b/libarchive/archive_read_support_format_rar.c |
| 41 | @@ -258,6 +258,7 @@ struct rar |
| 42 | struct data_block_offsets *dbo; |
| 43 | unsigned int cursor; |
| 44 | unsigned int nodes; |
| 45 | + char filename_must_match; |
| 46 | |
| 47 | /* LZSS members */ |
| 48 | struct huffman_code maincode; |
| 49 | @@ -1560,6 +1561,12 @@ read_header(struct archive_read *a, struct archive_entry *entry, |
| 50 | } |
| 51 | return ret; |
| 52 | } |
| 53 | + else if (rar->filename_must_match) |
| 54 | + { |
| 55 | + archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT, |
| 56 | + "Mismatch of file parts split across multi-volume archive"); |
| 57 | + return (ARCHIVE_FATAL); |
| 58 | + } |
| 59 | |
| 60 | rar->filename_save = (char*)realloc(rar->filename_save, |
| 61 | filename_size + 1); |
| 62 | @@ -2933,12 +2940,14 @@ rar_read_ahead(struct archive_read *a, size_t min, ssize_t *avail) |
| 63 | else if (*avail == 0 && rar->main_flags & MHD_VOLUME && |
| 64 | rar->file_flags & FHD_SPLIT_AFTER) |
| 65 | { |
| 66 | + rar->filename_must_match = 1; |
| 67 | ret = archive_read_format_rar_read_header(a, a->entry); |
| 68 | if (ret == (ARCHIVE_EOF)) |
| 69 | { |
| 70 | rar->has_endarc_header = 1; |
| 71 | ret = archive_read_format_rar_read_header(a, a->entry); |
| 72 | } |
| 73 | + rar->filename_must_match = 0; |
| 74 | if (ret != (ARCHIVE_OK)) |
| 75 | return NULL; |
| 76 | return rar_read_ahead(a, min, avail); |
| 77 | -- |
| 78 | 2.20.0 |
| 79 | |