From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id 149513858400 for ; Sun, 19 Dec 2021 20:23:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 149513858400 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: from reform (deer0x12.wildebeest.org [172.31.17.148]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 8287E302FBA8; Sun, 19 Dec 2021 21:23:23 +0100 (CET) Received: by reform (Postfix, from userid 1000) id 26A412E8374A; Sun, 19 Dec 2021 21:23:23 +0100 (CET) From: Mark Wielaard To: elfutils-devel@sourceware.org Cc: Mark Wielaard Subject: [PATCH] libdwfl: Rewrite GElf_Nhdr reading in dwfl_segment_report_module Date: Sun, 19 Dec 2021 21:23:21 +0100 Message-Id: <20211219202321.962702-1-mark@klomp.org> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Elfutils-devel mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 19 Dec 2021 20:23:29 -0000 Make sure that the notes filesz is not too big. Rewrite reading of the notes to check for overflow at every step. Also limit the size of the buildid bytes. Signed-off-by: Mark Wielaard --- libdwfl/ChangeLog | 5 ++ libdwfl/dwfl_segment_report_module.c | 79 ++++++++++++++++------------ 2 files changed, 49 insertions(+), 35 deletions(-) diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog index 1f83576d..18ffc347 100644 --- a/libdwfl/ChangeLog +++ b/libdwfl/ChangeLog @@ -1,3 +1,8 @@ +2021-12-19 Mark Wielaard + + * dwfl_segment_report_module.c (dwfl_segment_report_module): Check + notes filesz. Rewrite reading of GElf_Nhdr. + 2021-12-08 Mark Wielaard * dwfl_segment_report_module.c (dwfl_segment_report_module): Make sure diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c index 78c70795..a6d6be85 100644 --- a/libdwfl/dwfl_segment_report_module.c +++ b/libdwfl/dwfl_segment_report_module.c @@ -520,6 +520,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name, if (data_size != 0) filesz = data_size; + if (filesz > SIZE_MAX / sizeof (Elf32_Nhdr)) + continue; + assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr)); void *notes; @@ -532,6 +535,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name, { const unsigned int xencoding = ehdr.e32.e_ident[EI_DATA]; + if (filesz > SIZE_MAX / sizeof (Elf32_Nhdr)) + continue; notes = malloc (filesz); if (unlikely (notes == NULL)) continue; /* Next header */ @@ -552,44 +557,48 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name, const GElf_Nhdr *nh = notes; size_t len = 0; - size_t last_len; - while (filesz > len + sizeof (*nh)) + while (filesz - len > sizeof (*nh)) { - const void *note_name; - const void *note_desc; - last_len = len; - - len += sizeof (*nh); - note_name = notes + len; - - len += nh->n_namesz; - len = align == 8 ? NOTE_ALIGN8 (len) : NOTE_ALIGN4 (len); - note_desc = notes + len; - - if (unlikely (filesz < len + nh->n_descsz - || len <= last_len - || len + nh->n_descsz < last_len)) - break; - - if (nh->n_type == NT_GNU_BUILD_ID - && nh->n_descsz > 0 - && nh->n_namesz == sizeof "GNU" - && !memcmp (note_name, "GNU", sizeof "GNU")) - { - build_id.vaddr = (note_desc + len += sizeof (*nh); + + size_t namesz = nh->n_namesz; + namesz = align == 8 ? NOTE_ALIGN8 (namesz) : NOTE_ALIGN4 (namesz); + if (namesz > filesz - len || len + namesz < namesz) + break; + + void *note_name = notes + len; + len += namesz; + + size_t descsz = nh->n_descsz; + descsz = align == 8 ? NOTE_ALIGN8 (descsz) : NOTE_ALIGN4 (descsz); + if (descsz > filesz - len || len + descsz < descsz) + break; + + void *note_desc = notes + len; + len += descsz; + + /* We don't handle very short or really large build-ids. We need at + at least 3 and allow for up to 64 (normally ids are 20 long). */ +#define MIN_BUILD_ID_BYTES 3 +#define MAX_BUILD_ID_BYTES 64 + if (nh->n_type == NT_GNU_BUILD_ID + && nh->n_descsz >= MIN_BUILD_ID_BYTES + && nh->n_descsz <= MAX_BUILD_ID_BYTES + && nh->n_namesz == sizeof "GNU" + && !memcmp (note_name, "GNU", sizeof "GNU")) + { + build_id.vaddr = (note_desc - (const void *) notes + note_vaddr); - build_id.len = nh->n_descsz; - build_id.memory = malloc (build_id.len); - if (likely (build_id.memory != NULL)) - memcpy (build_id.memory, note_desc, build_id.len); - break; - } - - len += nh->n_descsz; - len = align == 8 ? NOTE_ALIGN8 (len) : NOTE_ALIGN4 (len); - nh = (void *) notes + len; - } + build_id.len = nh->n_descsz; + build_id.memory = malloc (build_id.len); + if (likely (build_id.memory != NULL)) + memcpy (build_id.memory, note_desc, build_id.len); + break; + } + + nh = (void *) notes + len; + } if (notes != data) free (notes); -- 2.30.2