Skip to content

Commit a0663a6

Browse files
authored
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.
1 parent c6aca87 commit a0663a6

File tree

1 file changed

+14
-10
lines changed

1 file changed

+14
-10
lines changed

src/modulefinder/sentry_modulefinder_linux.c

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -303,10 +303,11 @@ get_code_id_from_program_header(const sentry_module_t *module, size_t *size_out)
303303
Elf64_Ehdr elf;
304304
ENSURE(sentry__module_read_safely(&elf, module, 0, sizeof(Elf64_Ehdr)));
305305

306-
for (int i = 0; i < elf.e_phnum; i++) {
306+
for (uint32_t i = 0; i < elf.e_phnum; i++) {
307307
Elf64_Phdr header;
308308
ENSURE(sentry__module_read_safely(&header, module,
309-
elf.e_phoff + elf.e_phentsize * i, sizeof(Elf64_Phdr)));
309+
elf.e_phoff + (uint64_t)elf.e_phentsize * i,
310+
sizeof(Elf64_Phdr)));
310311

311312
// we are only interested in notes
312313
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)
326327
Elf32_Ehdr elf;
327328
ENSURE(sentry__module_read_safely(&elf, module, 0, sizeof(Elf32_Ehdr)));
328329

329-
for (int i = 0; i < elf.e_phnum; i++) {
330+
for (uint32_t i = 0; i < elf.e_phnum; i++) {
330331
Elf32_Phdr header;
331332
ENSURE(sentry__module_read_safely(&header, module,
332-
elf.e_phoff + elf.e_phentsize * i, sizeof(Elf32_Phdr)));
333+
elf.e_phoff + (uint64_t)elf.e_phentsize * i,
334+
sizeof(Elf32_Phdr)));
333335

334336
// we are only interested in notes
335337
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)
360362
\
361363
Elf64_Shdr strheader; \
362364
ENSURE(sentry__module_read_safely(&strheader, module, \
363-
elf.e_shoff + elf.e_shentsize * elf.e_shstrndx, \
365+
elf.e_shoff + (uint64_t)elf.e_shentsize * elf.e_shstrndx, \
364366
sizeof(Elf64_Shdr))); \
365367
\
366-
for (int i = 0; i < elf.e_shnum; i++) { \
368+
for (uint32_t i = 0; i < elf.e_shnum; i++) { \
367369
Elf64_Shdr header; \
368370
ENSURE(sentry__module_read_safely(&header, module, \
369-
elf.e_shoff + elf.e_shentsize * i, sizeof(Elf64_Shdr))); \
371+
elf.e_shoff + (uint64_t)elf.e_shentsize * i, \
372+
sizeof(Elf64_Shdr))); \
370373
\
371374
char name[6]; \
372375
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)
382385
\
383386
Elf32_Shdr strheader; \
384387
ENSURE(sentry__module_read_safely(&strheader, module, \
385-
elf.e_shoff + elf.e_shentsize * elf.e_shstrndx, \
388+
elf.e_shoff + (uint64_t)elf.e_shentsize * elf.e_shstrndx, \
386389
sizeof(Elf32_Shdr))); \
387390
\
388-
for (int i = 0; i < elf.e_shnum; i++) { \
391+
for (uint32_t i = 0; i < elf.e_shnum; i++) { \
389392
Elf32_Shdr header; \
390393
ENSURE(sentry__module_read_safely(&header, module, \
391-
elf.e_shoff + elf.e_shentsize * i, sizeof(Elf32_Shdr))); \
394+
elf.e_shoff + (uint64_t)elf.e_shentsize * i, \
395+
sizeof(Elf32_Shdr))); \
392396
\
393397
char name[6]; \
394398
ENSURE(sentry__module_read_safely(name, module, \

0 commit comments

Comments
 (0)