From a0663a67db1e7e019256d348f0a1ade935861f39 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 28 Oct 2024 14:31:23 +0100 Subject: [PATCH] fix: remove a potential overflow before conversion (#1062) * fix: remove a potential overflow before conversion This is in response to CodeQL security scan alert #1-#3. `Elf[32|64]_Ehdr[.e_phnum|.e_phentsize|.e_shnum|.e_shentsize]` are all `uint16_t`, this means the loop-var `i` is bounded by `uint16_t` and should fit in a `uint32_t` (to prevent unsigned overflow in the loop). A switch to unsigned still makes sense, because we reduce the future chance of unnecessary signed overflow (=UB) in the loop body. All program/section-header table entry sizes are cast to `uin64_t` even though the multiplication is bound to `uint32_t` by both factors being bound by `uint16_t`. This fixes the potential overflow before conversion to the bigger type. * also safely cast the access to section header string table. --- src/modulefinder/sentry_modulefinder_linux.c | 24 ++++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/modulefinder/sentry_modulefinder_linux.c b/src/modulefinder/sentry_modulefinder_linux.c index 1ae8e2b73..3af1a3eed 100644 --- a/src/modulefinder/sentry_modulefinder_linux.c +++ b/src/modulefinder/sentry_modulefinder_linux.c @@ -303,10 +303,11 @@ get_code_id_from_program_header(const sentry_module_t *module, size_t *size_out) Elf64_Ehdr elf; ENSURE(sentry__module_read_safely(&elf, module, 0, sizeof(Elf64_Ehdr))); - for (int i = 0; i < elf.e_phnum; i++) { + for (uint32_t i = 0; i < elf.e_phnum; i++) { Elf64_Phdr header; ENSURE(sentry__module_read_safely(&header, module, - elf.e_phoff + elf.e_phentsize * i, sizeof(Elf64_Phdr))); + elf.e_phoff + (uint64_t)elf.e_phentsize * i, + sizeof(Elf64_Phdr))); // we are only interested in notes if (header.p_type != PT_NOTE) { @@ -326,10 +327,11 @@ get_code_id_from_program_header(const sentry_module_t *module, size_t *size_out) Elf32_Ehdr elf; ENSURE(sentry__module_read_safely(&elf, module, 0, sizeof(Elf32_Ehdr))); - for (int i = 0; i < elf.e_phnum; i++) { + for (uint32_t i = 0; i < elf.e_phnum; i++) { Elf32_Phdr header; ENSURE(sentry__module_read_safely(&header, module, - elf.e_phoff + elf.e_phentsize * i, sizeof(Elf32_Phdr))); + elf.e_phoff + (uint64_t)elf.e_phentsize * i, + sizeof(Elf32_Phdr))); // we are only interested in notes if (header.p_type != PT_NOTE) { @@ -360,13 +362,14 @@ get_code_id_from_program_header(const sentry_module_t *module, size_t *size_out) \ Elf64_Shdr strheader; \ ENSURE(sentry__module_read_safely(&strheader, module, \ - elf.e_shoff + elf.e_shentsize * elf.e_shstrndx, \ + elf.e_shoff + (uint64_t)elf.e_shentsize * elf.e_shstrndx, \ sizeof(Elf64_Shdr))); \ \ - for (int i = 0; i < elf.e_shnum; i++) { \ + for (uint32_t i = 0; i < elf.e_shnum; i++) { \ Elf64_Shdr header; \ ENSURE(sentry__module_read_safely(&header, module, \ - elf.e_shoff + elf.e_shentsize * i, sizeof(Elf64_Shdr))); \ + elf.e_shoff + (uint64_t)elf.e_shentsize * i, \ + sizeof(Elf64_Shdr))); \ \ char name[6]; \ ENSURE(sentry__module_read_safely(name, module, \ @@ -382,13 +385,14 @@ get_code_id_from_program_header(const sentry_module_t *module, size_t *size_out) \ Elf32_Shdr strheader; \ ENSURE(sentry__module_read_safely(&strheader, module, \ - elf.e_shoff + elf.e_shentsize * elf.e_shstrndx, \ + elf.e_shoff + (uint64_t)elf.e_shentsize * elf.e_shstrndx, \ sizeof(Elf32_Shdr))); \ \ - for (int i = 0; i < elf.e_shnum; i++) { \ + for (uint32_t i = 0; i < elf.e_shnum; i++) { \ Elf32_Shdr header; \ ENSURE(sentry__module_read_safely(&header, module, \ - elf.e_shoff + elf.e_shentsize * i, sizeof(Elf32_Shdr))); \ + elf.e_shoff + (uint64_t)elf.e_shentsize * i, \ + sizeof(Elf32_Shdr))); \ \ char name[6]; \ ENSURE(sentry__module_read_safely(name, module, \