From c717c7789b4f79a4cb500f9287f5d355fed591a4 Mon Sep 17 00:00:00 2001 From: Pete Batard Date: Thu, 29 Jun 2023 21:15:35 +0100 Subject: [PATCH] [cmp] further ZIP64 and overall Bled improvements * The ZIP64 extra record may not be the first one, so add processing for all extra zip records. * Also add extra sanity checks to try to appease Coverity and properly detect short writes. --- src/bled/decompress_gunzip.c | 6 ++- src/bled/decompress_unzip.c | 92 +++++++++++++++++++++++++----------- src/bled/decompress_vtsi.c | 1 - src/bled/libbb.h | 40 +++++++++++----- src/rufus.rc | 10 ++-- 5 files changed, 102 insertions(+), 47 deletions(-) diff --git a/src/bled/decompress_gunzip.c b/src/bled/decompress_gunzip.c index 52f12cb6..94b4b374 100644 --- a/src/bled/decompress_gunzip.c +++ b/src/bled/decompress_gunzip.c @@ -1197,9 +1197,13 @@ static int check_header_gzip(STATE_PARAM transformer_state_t *xstate) if (header.formatted.flags & 0x18) { while (1) { do { + /* None of our buffers should be larger than BB_BUFSIZE */ + if (bytebuffer_offset > BB_BUFSIZE) { + bb_error_msg("buffer overflow"); + return 0; + } if (!top_up(PASS_STATE 1)) return 0; - // coverity[tainted_data] } while (bytebuffer[bytebuffer_offset++] != 0); if ((header.formatted.flags & 0x18) != 0x18) break; diff --git a/src/bled/decompress_unzip.c b/src/bled/decompress_unzip.c index 186df546..fffdf3d2 100644 --- a/src/bled/decompress_unzip.c +++ b/src/bled/decompress_unzip.c @@ -140,6 +140,26 @@ do { if (BB_BIG_ENDIAN) { \ (cde).fmt.cdf_offset = SWAP_LE32((cde).fmt.cdf_offset); \ }} while (0) +/* Extra records */ +#define EXTRA_HEADER_LEN 4 + +PRAGMA_BEGIN_PACKED +typedef union { + uint8_t raw[EXTRA_HEADER_LEN]; + struct { + uint16_t tag; + uint16_t length; + /* extra record follows */ + } fmt PACKED; +} extra_header_t; +PRAGMA_END_PACKED + +#define FIX_ENDIANNESS_EXTRA(ext) \ +do { if (BB_BIG_ENDIAN) { \ + (ext).fmt.tag = SWAP_LE16((ext).fmt.tag); \ + (ext).fmt.length = SWAP_LE32((ext).fmt.length); \ +}} while (0) + /* ZIP64 records */ #define ZIP64_LEN 20 @@ -147,8 +167,8 @@ PRAGMA_BEGIN_PACKED typedef union { uint8_t raw[ZIP64_LEN]; struct { - /* uint16_t signature; 00 01 */ - uint16_t signature; + /* uint16_t tag; 00 01 */ + uint16_t tag; uint16_t length; uint64_t ucmpsize PACKED; uint64_t cmpsize PACKED; @@ -158,8 +178,6 @@ PRAGMA_END_PACKED #define FIX_ENDIANNESS_ZIP64(z64) \ do { if (BB_BIG_ENDIAN) { \ - (z64).fmt.signature = SWAP_LE16((z64).fmt.signature); \ - (z64).fmt.length = SWAP_LE32((z64).fmt.length); \ (z64).fmt.cmpsize = SWAP_LE64((z64).fmt.cmpsize); \ (z64).fmt.ucmpsize = SWAP_LE64((z64).fmt.ucmpsize); \ }} while (0) @@ -191,25 +209,22 @@ do { if (BB_BIG_ENDIAN) { \ (c64).fmt.cdf_offset = SWAP_LE64((c64).fmt.cdf_offset); \ }} while (0) -struct BUG { +inline void BUG() { /* Check the offset of the last element, not the length. This leniency * allows for poor packing, whereby the overall struct may be too long, * even though the elements are all in the right place. */ - char BUG_zip_header_must_be_26_bytes[ + BUILD_BUG_ON( offsetof(zip_header_t, fmt.extra_len) + 2 - == ZIP_HEADER_LEN ? 1 : -1]; - char BUG_cdf_header_must_be_42_bytes[ + != ZIP_HEADER_LEN); + BUILD_BUG_ON( offsetof(cdf_header_t, fmt.relative_offset_of_local_header) + 4 - == CDF_HEADER_LEN ? 1 : -1]; - char BUG_cde_must_be_16_bytes[ - sizeof(cde_t) == CDE_LEN ? 1 : -1]; - char BUG_zip64_must_be_20_bytes[ - sizeof(zip64_t) == ZIP64_LEN ? 1 : -1]; - char BUG_cde64_must_be_52_bytes[ - sizeof(cde64_t) == CDE64_LEN ? 1 : -1]; -}; - + != CDF_HEADER_LEN); + BUILD_BUG_ON(sizeof(cde_t) != CDE_LEN); + BUILD_BUG_ON(sizeof(extra_header_t) != EXTRA_HEADER_LEN); + BUILD_BUG_ON(sizeof(zip64_t) != ZIP64_LEN); + BUILD_BUG_ON(sizeof(cde64_t) != CDE64_LEN); +} /* This value means that we failed to find CDF */ #define BAD_CDF_OFFSET ((uint32_t)0xffffffff) @@ -381,6 +396,8 @@ static void unzip_skip(int fd, off_t skip) static void unzip_set_xstate(transformer_state_t* xstate, zip_header_t* zip) { uint8_t* buf = NULL; + uint16_t i = 0; + extra_header_t* extra; zip64_t* zip64; /* Set the default sizes for non ZIP64 content */ @@ -396,20 +413,39 @@ static void unzip_set_xstate(transformer_state_t* xstate, zip_header_t* zip) xstate->dst_name[zip->fmt.filename_len] = 0; /* Read the extra data */ - if (zip->fmt.extra_len != 0) { + if (zip->fmt.extra_len) { + dbg("Reading extra data"); buf = malloc(zip->fmt.extra_len); if (buf == NULL) goto err; xread(xstate->src_fd, buf, zip->fmt.extra_len); } - /* Process the ZIP64 data */ - zip64 = (zip64_t*)buf; - FIX_ENDIANNESS_ZIP64(*zip64); - if (zip->fmt.cmpsize == 0xffffffffL && zip->fmt.ucmpsize == 0xffffffffL && - zip->fmt.extra_len >= sizeof(zip64_t) && zip64->fmt.signature == SWAP_LE16(0x0001)) { - xstate->dst_size = zip64->fmt.ucmpsize; - xstate->bytes_in = zip64->fmt.cmpsize; + /* Process the extra records */ + if (zip->fmt.extra_len < EXTRA_HEADER_LEN + 1) + goto err; + for (i = 0; i < zip->fmt.extra_len - EXTRA_HEADER_LEN; ) { + extra = (extra_header_t*)&buf[i]; + FIX_ENDIANNESS_EXTRA(*extra); + dbg(" tag:0x%04x len:%u", + (unsigned)extra->fmt.tag, + (unsigned)extra->fmt.length + ); + i += EXTRA_HEADER_LEN + extra->fmt.length; + /* Process the ZIP64 data */ + if (extra->fmt.tag == SWAP_LE16(0x0001)) { + zip64 = (zip64_t*)extra; + FIX_ENDIANNESS_ZIP64(*zip64); + if ((zip->fmt.cmpsize == 0xffffffffL || zip->fmt.ucmpsize == 0xffffffffL) && + EXTRA_HEADER_LEN + zip64->fmt.length >= ZIP64_LEN) { + dbg("Actual cmpsize:0x%"OFF_FMT"x ucmpsize:0x%"OFF_FMT"x", + zip64->fmt.cmpsize, + zip64->fmt.ucmpsize + ); + xstate->dst_size = zip64->fmt.ucmpsize; + xstate->bytes_in = zip64->fmt.cmpsize; + } + } } err: @@ -448,7 +484,7 @@ unzip_extract(zip_header_t* zip, transformer_state_t* xstate) * and will seek to the correct beginning of next file. */ xstate->bytes_out = unpack_bz2_stream(xstate); - if (xstate->bytes_out < 0) + if ((int64_t)xstate->bytes_out < 0) bb_simple_error_msg_and_die("inflate error"); } #endif @@ -456,7 +492,7 @@ unzip_extract(zip_header_t* zip, transformer_state_t* xstate) else if (zip->fmt.method == 14) { /* Not tested yet */ xstate->bytes_out = unpack_lzma_stream(xstate); - if (xstate->bytes_out < 0) + if ((int64_t)xstate->bytes_out < 0) bb_simple_error_msg_and_die("inflate error"); } #endif @@ -464,7 +500,7 @@ unzip_extract(zip_header_t* zip, transformer_state_t* xstate) else if (zip->fmt.method == 95) { /* Not tested yet */ xstate->bytes_out = unpack_xz_stream(xstate); - if (xstate->bytes_out < 0) + if ((int64_t)xstate->bytes_out < 0) bb_simple_error_msg_and_die("inflate error"); } #endif diff --git a/src/bled/decompress_vtsi.c b/src/bled/decompress_vtsi.c index 42ae6072..70a2c269 100644 --- a/src/bled/decompress_vtsi.c +++ b/src/bled/decompress_vtsi.c @@ -149,7 +149,6 @@ IF_DESKTOP(long long) int FAST_FUNC unpack_vtsi_stream(transformer_state_t* xsta buf = (uint8_t*)segment + footer.segment_num * sizeof(VTSI_SEGMENT); lseek(src_fd, footer.segment_offset, SEEK_SET); - /* coverity[tainted_data_argument] */ safe_read(src_fd, segment, footer.segment_num * sizeof(VTSI_SEGMENT)); if (!check_vtsi_segment(&footer, segment)) diff --git a/src/bled/libbb.h b/src/bled/libbb.h index 983c7c6d..836e63dc 100644 --- a/src/bled/libbb.h +++ b/src/bled/libbb.h @@ -39,24 +39,25 @@ #include #include -#define BB_BUFSIZE 0x40000 +#define BB_BUFSIZE 0x40000 +#define ONE_TB 1099511627776ULL -#define ENABLE_DESKTOP 1 +#define ENABLE_DESKTOP 1 #if ENABLE_DESKTOP -#define IF_DESKTOP(x) x +#define IF_DESKTOP(x) x #define IF_NOT_DESKTOP(x) #else #define IF_DESKTOP(x) -#define IF_NOT_DESKTOP(x) x +#define IF_NOT_DESKTOP(x) x #endif #define IF_NOT_FEATURE_LZMA_FAST(x) x -#define ENABLE_FEATURE_UNZIP_CDF 1 -#define ENABLE_FEATURE_UNZIP_BZIP2 1 -#define ENABLE_FEATURE_UNZIP_LZMA 1 -#define ENABLE_FEATURE_UNZIP_XZ 1 +#define ENABLE_FEATURE_UNZIP_CDF 1 +#define ENABLE_FEATURE_UNZIP_BZIP2 1 +#define ENABLE_FEATURE_UNZIP_LZMA 1 +#define ENABLE_FEATURE_UNZIP_XZ 1 -#define uoff_t unsigned off_t -#define OFF_FMT "ll" +#define uoff_t unsigned off_t +#define OFF_FMT "ll" #ifndef _MODE_T_ #define _MODE_T_ @@ -147,7 +148,7 @@ extern unsigned long* bled_cancel_request; #define xfunc_die() longjmp(bb_error_jmp, 1) #define bb_printf(...) do { if (bled_printf != NULL) bled_printf(__VA_ARGS__); \ else { printf(__VA_ARGS__); putchar('\n'); } } while(0) -#define bb_error_msg(...) bb_printf("Error: " __VA_ARGS__) +#define bb_error_msg(...) bb_printf("\nError: " __VA_ARGS__) #define bb_error_msg_and_die(...) do {bb_error_msg(__VA_ARGS__); xfunc_die();} while(0) #define bb_error_msg_and_err(...) do {bb_error_msg(__VA_ARGS__); goto err;} while(0) #define bb_perror_msg bb_error_msg @@ -191,6 +192,11 @@ static inline int full_read(int fd, void *buf, unsigned int count) { errno = EFAULT; return -1; } + /* None of our r/w buffers should be larger than BB_BUFSIZE */ + if (count > BB_BUFSIZE) { + errno = E2BIG; + return -1; + } if ((bled_cancel_request != NULL) && (*bled_cancel_request != 0)) { errno = EINTR; return -1; @@ -215,6 +221,12 @@ static inline int full_read(int fd, void *buf, unsigned int count) { static inline int full_write(int fd, const void* buffer, unsigned int count) { + /* None of our r/w buffers should be larger than BB_BUFSIZE */ + if (count > BB_BUFSIZE) { + errno = E2BIG; + return -1; + } + return (bled_write != NULL) ? bled_write(fd, buffer, count) : _write(fd, buffer, count); } @@ -226,6 +238,10 @@ static inline void bb_copyfd_exact_size(int fd1, int fd2, off_t size) if (fd1 < 0 || fd2 < 0) bb_error_msg_and_die("invalid fd"); + /* Enforce a 1 TB limit to keep Coverity happy */ + if (size > ONE_TB) + bb_error_msg_and_die("too large"); + buf = malloc(BB_BUFSIZE); if (buf == NULL) bb_error_msg_and_die("out of memory"); @@ -246,7 +262,7 @@ static inline void bb_copyfd_exact_size(int fd1, int fd2, off_t size) free(buf); bb_error_msg_and_die("write error"); } - if (w == 0) { + if (w != r) { bb_error_msg("short write"); break; } diff --git a/src/rufus.rc b/src/rufus.rc index 1131f02f..c23564ad 100644 --- a/src/rufus.rc +++ b/src/rufus.rc @@ -33,7 +33,7 @@ LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL IDD_DIALOG DIALOGEX 12, 12, 232, 326 STYLE DS_SETFONT | DS_MODALFRAME | DS_CENTER | WS_MINIMIZEBOX | WS_POPUP | WS_CAPTION | WS_SYSMENU EXSTYLE WS_EX_ACCEPTFILES -CAPTION "Rufus 4.2.2056" +CAPTION "Rufus 4.2.2057" FONT 9, "Segoe UI Symbol", 400, 0, 0x0 BEGIN LTEXT "Drive Properties",IDS_DRIVE_PROPERTIES_TXT,8,6,53,12,NOT WS_GROUP @@ -392,8 +392,8 @@ END // VS_VERSION_INFO VERSIONINFO - FILEVERSION 4,2,2056,0 - PRODUCTVERSION 4,2,2056,0 + FILEVERSION 4,2,2057,0 + PRODUCTVERSION 4,2,2057,0 FILEFLAGSMASK 0x3fL #ifdef _DEBUG FILEFLAGS 0x1L @@ -411,13 +411,13 @@ BEGIN VALUE "Comments", "https://rufus.ie" VALUE "CompanyName", "Akeo Consulting" VALUE "FileDescription", "Rufus" - VALUE "FileVersion", "4.2.2056" + VALUE "FileVersion", "4.2.2057" VALUE "InternalName", "Rufus" VALUE "LegalCopyright", "© 2011-2023 Pete Batard (GPL v3)" VALUE "LegalTrademarks", "https://www.gnu.org/licenses/gpl-3.0.html" VALUE "OriginalFilename", "rufus-4.2.exe" VALUE "ProductName", "Rufus" - VALUE "ProductVersion", "4.2.2056" + VALUE "ProductVersion", "4.2.2057" END END BLOCK "VarFileInfo"