Skip to content

Commit

Permalink
fix: remove a potential overflow before conversion (getsentry#1062)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@supervacuus
supervacuus authored Oct 28, 2024
1 parent c6aca87 commit a0663a6
Showing 1 changed file with 14 additions and 10 deletions.
24 changes: 14 additions & 10 deletions src/modulefinder/sentry_modulefinder_linux.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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, \
Expand All @@ -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, \
Expand Down

0 comments on commit a0663a6

Please sign in to comment.