From 634b886e84a5c568d243e744becc6b3223e089cf Mon Sep 17 00:00:00 2001 From: pancake Date: Wed, 23 Feb 2022 22:54:54 +0100 Subject: [PATCH] Fix DoS in PE/QNX/DYLDCACHE/PSX parsers ##crash * Reported by lazymio * Reproducer: AAA4AAAAAB4= --- libr/bin/format/pe/pe.c | 18 ++++++++---------- libr/bin/p/bin_dyldcache.c | 2 +- libr/bin/p/bin_psxexe.c | 4 ++-- libr/bin/p/bin_qnx.c | 14 +++++++------- libr/util/buf.c | 6 ++---- 5 files changed, 20 insertions(+), 24 deletions(-) diff --git a/libr/bin/format/pe/pe.c b/libr/bin/format/pe/pe.c index e133451d08470..7718abebca8e0 100644 --- a/libr/bin/format/pe/pe.c +++ b/libr/bin/format/pe/pe.c @@ -1,4 +1,4 @@ -/* radare - LGPL - Copyright 2008-2021 nibble, pancake, inisider */ +/* radare - LGPL - Copyright 2008-2022 nibble, pancake, inisider */ #include #include @@ -278,15 +278,13 @@ struct r_bin_pe_addr_t *PE_(check_msvcseh)(RBinPEObj *pe) { } struct r_bin_pe_addr_t *PE_(check_mingw)(RBinPEObj *pe) { - struct r_bin_pe_addr_t* entry; bool sw = false; - ut8 b[1024]; + ut8 b[1024] = {0}; size_t n = 0; if (!pe || !pe->b) { return 0LL; } - entry = PE_(r_bin_pe_get_entrypoint) (pe); - ZERO_FILL (b); + struct r_bin_pe_addr_t* entry = PE_(r_bin_pe_get_entrypoint) (pe); if (r_buf_read_at (pe->b, entry->paddr, b, sizeof (b)) < 0) { pe_printf ("Warning: Cannot read entry at 0x%08"PFMT64x "\n", entry->paddr); free (entry); @@ -339,11 +337,11 @@ struct r_bin_pe_addr_t *PE_(check_unknow)(RBinPEObj *pe) { if (!pe || !pe->b) { return 0LL; } - ut8 b[512]; + ut8 b[512] = {0}; ZERO_FILL (b); entry = PE_ (r_bin_pe_get_entrypoint) (pe); // option2: /x 8bff558bec83ec20 - if (r_buf_read_at (pe->b, entry->paddr, b, 512) < 1) { + if (r_buf_read_at (pe->b, entry->paddr, b, sizeof (b)) != sizeof (b)) { pe_printf ("Warning: Cannot read entry at 0x%08"PFMT64x"\n", entry->paddr); free (entry); return NULL; @@ -537,7 +535,8 @@ static int bin_pe_parse_imports(RBinPEObj* pe, if (len < 1) { pe_printf ("Warning: read (import name)\n"); goto error; - } else if (!*name) { + } + if (!*name) { break; } name[PE_NAME_LENGTH] = '\0'; @@ -1258,7 +1257,6 @@ static bool bin_pe_init_metadata_hdr(RBinPEObj* pe) { // read the header after the string rr = r_buf_fread_at (pe->b, metadata_directory + 16 + metadata->VersionStringLength, (ut8*) (&metadata->Flags), pe->big_endian? "2S": "2s", 1); - if (rr < 1) { goto fail; } @@ -1820,7 +1818,7 @@ static Var* Pe_r_bin_pe_parse_var(RBinPEObj* pe, PE_DWord* curAddr) { free_Var (var); return NULL; } - if (r_buf_read_at (pe->b, *curAddr, (ut8*) var->szKey, TRANSLATION_UTF_16_LEN) < 1) { + if (r_buf_read_at (pe->b, *curAddr, (ut8*) var->szKey, TRANSLATION_UTF_16_LEN) != TRANSLATION_UTF_16_LEN) { pe_printf ("Warning: read (Var szKey)\n"); free_Var (var); return NULL; diff --git a/libr/bin/p/bin_dyldcache.c b/libr/bin/p/bin_dyldcache.c index 6b7b512b71365..d5700f1682151 100644 --- a/libr/bin/p/bin_dyldcache.c +++ b/libr/bin/p/bin_dyldcache.c @@ -587,7 +587,7 @@ static ut64 estimate_slide(RBinFile *bf, RDyldCache *cache, ut64 value_mask, ut6 int n_classes = classlist_sample_size / 8; ut64 sect_offset = sections[classlist_idx].offset + bin->hdr_offset; - if (r_buf_fread_at (cache->buf, sect_offset, (ut8*) classlist, "l", n_classes) < classlist_sample_size) { + if (r_buf_fread_at (cache->buf, sect_offset, (ut8*) classlist, "l", n_classes) != classlist_sample_size) { goto next_bin; } diff --git a/libr/bin/p/bin_psxexe.c b/libr/bin/p/bin_psxexe.c index 56e90384a863b..5865bf0677be9 100644 --- a/libr/bin/p/bin_psxexe.c +++ b/libr/bin/p/bin_psxexe.c @@ -57,7 +57,7 @@ static RList* sections(RBinFile* bf) { return NULL; } - if (r_buf_fread_at (bf->buf, 0, (ut8*)&psxheader, "8c17i", 1) < sizeof (psxexe_header)) { + if (r_buf_fread_at (bf->buf, 0, (ut8*)&psxheader, "8c17i", 1) != sizeof (psxexe_header)) { eprintf ("Truncated Header\n"); free (sect); r_list_free (ret); @@ -93,7 +93,7 @@ static RList* entries(RBinFile* bf) { return NULL; } - if (r_buf_fread_at (bf->buf, 0, (ut8*)&psxheader, "8c17i", 1) < sizeof (psxexe_header)) { + if (r_buf_fread_at (bf->buf, 0, (ut8*)&psxheader, "8c17i", 1) != sizeof (psxexe_header)) { eprintf ("PSXEXE Header truncated\n"); r_list_free (ret); free (addr); diff --git a/libr/bin/p/bin_qnx.c b/libr/bin/p/bin_qnx.c index c3e93c930a3c7..a00e2631faf43 100644 --- a/libr/bin/p/bin_qnx.c +++ b/libr/bin/p/bin_qnx.c @@ -7,7 +7,7 @@ static int lmf_header_load(lmf_header *lmfh, RBuffer *buf, Sdb *db) { if (r_buf_size (buf) < sizeof (lmf_header)) { return false; } - if (r_buf_fread_at (buf, QNX_HEADER_ADDR, (ut8 *) lmfh, "iiiiiiiicccciiiicc", 1) < QNX_HDR_SIZE) { + if (r_buf_fread_at (buf, QNX_HEADER_ADDR, (ut8 *) lmfh, "iiiiiiiicccciiiicc", 1) != QNX_HDR_SIZE) { return false; } r_strf_buffer (32); @@ -64,7 +64,7 @@ static bool load_buffer(RBinFile *bf, void **bin_obj, RBuffer *buf, ut64 loadadd goto beach; } // Read the first record - if (r_buf_fread_at (bf->buf, 0, (ut8 *)&lrec, "ccss", 1) < QNX_RECORD_SIZE) { + if (r_buf_fread_at (bf->buf, 0, (ut8 *)&lrec, "ccss", 1) != QNX_RECORD_SIZE) { goto beach; } // Load the header @@ -72,7 +72,7 @@ static bool load_buffer(RBinFile *bf, void **bin_obj, RBuffer *buf, ut64 loadadd offset += lrec.data_nbytes; for (;;) { - if (r_buf_fread_at (bf->buf, offset, (ut8 *)&lrec, "ccss", 1) < QNX_RECORD_SIZE) { + if (r_buf_fread_at (bf->buf, offset, (ut8 *)&lrec, "ccss", 1) != QNX_RECORD_SIZE) { goto beach; } offset += sizeof (lmf_record); @@ -84,7 +84,7 @@ static bool load_buffer(RBinFile *bf, void **bin_obj, RBuffer *buf, ut64 loadadd if (!ptr) { goto beach; } - if (r_buf_fread_at (bf->buf, offset, (ut8 *)&lres, "ssss", 1) < sizeof (lmf_resource)) { + if (r_buf_fread_at (bf->buf, offset, (ut8 *)&lres, "ssss", 1) != sizeof (lmf_resource)) { goto beach; } ptr->name = strdup ("LMF_RESOURCE"); @@ -95,7 +95,7 @@ static bool load_buffer(RBinFile *bf, void **bin_obj, RBuffer *buf, ut64 loadadd r_list_append (sections, ptr); } else if (lrec.rec_type == LMF_LOAD_REC) { RBinSection *ptr = R_NEW0 (RBinSection); - if (r_buf_fread_at (bf->buf, offset, (ut8 *)&ldata, "si", 1) < sizeof (lmf_data)) { + if (r_buf_fread_at (bf->buf, offset, (ut8 *)&ldata, "si", 1) != sizeof (lmf_data)) { goto beach; } if (!ptr) { @@ -110,7 +110,7 @@ static bool load_buffer(RBinFile *bf, void **bin_obj, RBuffer *buf, ut64 loadadd r_list_append (sections, ptr); } else if (lrec.rec_type == LMF_FIXUP_REC) { RBinReloc *ptr = R_NEW0 (RBinReloc); - if (!ptr || r_buf_fread_at (bf->buf, offset, (ut8 *)&ldata, "si", 1) < sizeof (lmf_data)) { + if (!ptr || r_buf_fread_at (bf->buf, offset, (ut8 *)&ldata, "si", 1) != sizeof (lmf_data)) { goto beach; } ptr->vaddr = ptr->paddr = ldata.offset; @@ -118,7 +118,7 @@ static bool load_buffer(RBinFile *bf, void **bin_obj, RBuffer *buf, ut64 loadadd r_list_append (fixups, ptr); } else if (lrec.rec_type == LMF_8087_FIXUP_REC) { RBinReloc *ptr = R_NEW0 (RBinReloc); - if (!ptr || r_buf_fread_at (bf->buf, offset, (ut8 *)&ldata, "si", 1) < sizeof (lmf_data)) { + if (!ptr || r_buf_fread_at (bf->buf, offset, (ut8 *)&ldata, "si", 1) != sizeof (lmf_data)) { goto beach; } ptr->vaddr = ptr->paddr = ldata.offset; diff --git a/libr/util/buf.c b/libr/util/buf.c index b20e481d4701e..5aa93ca64bc51 100644 --- a/libr/util/buf.c +++ b/libr/util/buf.c @@ -516,10 +516,9 @@ static st64 buf_format(RBuffer *dst, RBuffer *src, const char *fmt, int n) { ut32 d3; ut64 d4; st64 r = r_buf_read (src, tmp, tsize); - if (r < tsize) { + if (r != tsize) { return -1; } - switch (tsize) { case 1: d1 = r_read_ble8 (tmp); @@ -566,7 +565,7 @@ R_API st64 r_buf_fread_at(RBuffer *b, ut64 addr, ut8 *buf, const char *fmt, int return r; } r = r_buf_fread (b, buf, fmt, n); - r_buf_seek (b, o_addr, R_BUF_SET); + (void)r_buf_seek (b, o_addr, R_BUF_SET); return r; } @@ -598,7 +597,6 @@ R_API st64 r_buf_read_at(RBuffer *b, ut64 addr, ut8 *buf, ut64 len) { if (r < 0) { return r; } - r = r_buf_read (b, buf, len); r_buf_seek (b, o_addr, R_BUF_SET); return r;