From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1062) id C1E113846402; Thu, 4 Apr 2024 01:18:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C1E113846402 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1712193518; bh=dmhVKEw3zeTmL8q9T/X8aouN4hYccwJfxRBlc6valx4=; h=From:To:Subject:Date:From; b=m0RzUE15kG3LI6+r00ivX1bP4mHV9NP+JcWSzszseYdCX+CNKa5tXQ3b8nDg8R7Sw ShAQrDgXDCxpebcu5DvzhP385EwBDGAWjhZM3rZOaznIkO7nm+Ac0b37gP8YQ7kcD8 01c5l1YaghdpNTK0cbRUUEVv6TPNepme7THxXjYM= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Alan Modra To: binutils-cvs@sourceware.org Subject: [binutils-gdb] USE_MMAP fuzzed object file attacks X-Act-Checkin: binutils-gdb X-Git-Author: Alan Modra X-Git-Refname: refs/heads/master X-Git-Oldrev: 7e217ee2c06e6580386eccba812f767e20e61a00 X-Git-Newrev: b86d3af60ffc5a821aa54404f57ffe9476919135 Message-Id: <20240404011838.C1E113846402@sourceware.org> Date: Thu, 4 Apr 2024 01:18:38 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Db86d3af60ffc= 5a821aa54404f57ffe9476919135 commit b86d3af60ffc5a821aa54404f57ffe9476919135 Author: Alan Modra Date: Thu Apr 4 07:51:47 2024 +1030 USE_MMAP fuzzed object file attacks =20 If mmap is used without sanity checking, then we'll get a SIGBUS if an access is done to the mmap'd memory corresponding to a page past end of file. =20 * aoutx.h (aout_get_external_symbols): Check that mmap regions are within file contents. Catch stringsize overflow. (some_aout_object_p): Don't clear already zeroed fields. Tidy. * pdp11.c: As for aoutx.h. Copy some fixes too. Diff: --- bfd/aoutx.h | 65 +++++++++++++++++++++-------------------- bfd/pdp11.c | 96 ++++++++++++++++++++++++++++++++-------------------------= ---- 2 files changed, 84 insertions(+), 77 deletions(-) diff --git a/bfd/aoutx.h b/bfd/aoutx.h index c8aaa14baeb..fb6326d79d1 100644 --- a/bfd/aoutx.h +++ b/bfd/aoutx.h @@ -498,9 +498,9 @@ NAME (aout, some_aout_object_p) (bfd *abfd, { struct aout_data_struct *rawptr, *oldrawptr; bfd_cleanup result; - size_t amt =3D sizeof (* rawptr); + size_t amt =3D sizeof (*rawptr); =20 - rawptr =3D (struct aout_data_struct *) bfd_zalloc (abfd, amt); + rawptr =3D bfd_zalloc (abfd, amt); if (rawptr =3D=3D NULL) return NULL; =20 @@ -551,7 +551,6 @@ NAME (aout, some_aout_object_p) (bfd *abfd, =20 abfd->start_address =3D execp->a_entry; =20 - obj_aout_symbols (abfd) =3D NULL; abfd->symcount =3D execp->a_syms / sizeof (struct external_nlist); =20 /* The default relocation entry size is that of traditional V7 Unix. */ @@ -564,9 +563,6 @@ NAME (aout, some_aout_object_p) (bfd *abfd, bfd_init_window (&obj_aout_sym_window (abfd)); bfd_init_window (&obj_aout_string_window (abfd)); #endif - obj_aout_external_syms (abfd) =3D NULL; - obj_aout_external_strings (abfd) =3D NULL; - obj_aout_sym_hashes (abfd) =3D NULL; =20 if (! NAME (aout, make_sections) (abfd)) goto error_ret; @@ -594,7 +590,6 @@ NAME (aout, some_aout_object_p) (bfd *abfd, /* Call back to the format-dependent code to fill in the rest of the fields and do any further cleanup. Things that should be filled in by the callback: */ - struct exec *execp =3D exec_hdr (abfd); =20 obj_textsec (abfd)->size =3D N_TXTSIZE (execp); @@ -618,18 +613,13 @@ NAME (aout, some_aout_object_p) (bfd *abfd, obj_sym_filepos (abfd) =3D N_SYMOFF (execp); =20 /* Determine the architecture and machine type of the object file. */ - switch (N_MACHTYPE (exec_hdr (abfd))) - { - default: - abfd->obj_arch =3D bfd_arch_obscure; - break; - } + abfd->obj_arch =3D bfd_arch_obscure; =20 adata (abfd)->page_size =3D TARGET_PAGE_SIZE; adata (abfd)->segment_size =3D SEGMENT_SIZE; adata (abfd)->exec_bytes_size =3D EXEC_BYTES_SIZE; =20 - return _bfd_no_cleanup + return _bfd_no_cleanup; =20 /* The architecture is encoded in various ways in various a.out variants, or is not encoded at all in some of them. The relocation size depends @@ -639,7 +629,7 @@ NAME (aout, some_aout_object_p) (bfd *abfd, =20 Formats such as b.out, which have additional fields in the a.out header, should cope with them in this callback as well. */ -#endif /* DOCUMENTATION */ +#endif /* DOCUMENTATION */ =20 result =3D (*callback_to_real_object_p) (abfd); =20 @@ -1311,30 +1301,41 @@ NAME (aout, set_section_contents) (bfd *abfd, static bool aout_get_external_symbols (bfd *abfd) { +#ifdef USE_MMAP + ufile_ptr filesize =3D bfd_get_file_size (abfd); +#endif + if (obj_aout_external_syms (abfd) =3D=3D NULL) { bfd_size_type count; - struct external_nlist *syms; + struct external_nlist *syms =3D NULL; bfd_size_type amt =3D exec_hdr (abfd)->a_syms; =20 count =3D amt / EXTERNAL_NLIST_SIZE; if (count =3D=3D 0) - return true; /* Nothing to do. */ + return true; =20 #ifdef USE_MMAP - if (! bfd_get_file_window (abfd, obj_sym_filepos (abfd), amt, - &obj_aout_sym_window (abfd), true)) - return false; - syms =3D (struct external_nlist *) obj_aout_sym_window (abfd).data; + if (filesize >=3D (ufile_ptr) obj_sym_filepos (abfd) + && filesize - obj_sym_filepos (abfd) >=3D amt) + { + if (! bfd_get_file_window (abfd, obj_sym_filepos (abfd), amt, + &obj_aout_sym_window (abfd), true)) + return false; + syms =3D (struct external_nlist *) obj_aout_sym_window (abfd).data; + } + else #else - /* We allocate using malloc to make the values easy to free - later on. If we put them on the objalloc it might not be - possible to free them. */ - if (bfd_seek (abfd, obj_sym_filepos (abfd), SEEK_SET) !=3D 0) - return false; - syms =3D (struct external_nlist *) _bfd_malloc_and_read (abfd, amt, = amt); - if (syms =3D=3D NULL) - return false; + { + /* We allocate using malloc to make the values easy to free + later on. If we put them on the objalloc it might not be + possible to free them. */ + if (bfd_seek (abfd, obj_sym_filepos (abfd), SEEK_SET) !=3D 0) + return false; + syms =3D (struct external_nlist *) _bfd_malloc_and_read (abfd, amt, amt= ); + if (syms =3D=3D NULL) + return false; + } #endif =20 obj_aout_external_syms (abfd) =3D syms; @@ -1356,7 +1357,7 @@ aout_get_external_symbols (bfd *abfd) stringsize =3D GET_WORD (abfd, string_chars); if (stringsize =3D=3D 0) stringsize =3D 1; - else if (stringsize < BYTES_IN_WORD + else if (stringsize + 1 < BYTES_IN_WORD + 1 || (size_t) stringsize !=3D stringsize) { bfd_set_error (bfd_error_bad_value); @@ -1364,7 +1365,9 @@ aout_get_external_symbols (bfd *abfd) } =20 #ifdef USE_MMAP - if (stringsize >=3D BYTES_IN_WORD) + if (stringsize >=3D BYTES_IN_WORD + && filesize >=3D (ufile_ptr) obj_str_filepos (abfd) + && filesize - obj_str_filepos (abfd) >=3D stringsize + 1) { if (! bfd_get_file_window (abfd, obj_str_filepos (abfd), stringsize + 1, &obj_aout_string_window (abfd), true)) diff --git a/bfd/pdp11.c b/bfd/pdp11.c index e83a4854aa5..f9ded64c933 100644 --- a/bfd/pdp11.c +++ b/bfd/pdp11.c @@ -534,12 +534,12 @@ NAME (aout, some_aout_object_p) (bfd *abfd, bfd_cleanup (*callback_to_real_object_p) (bfd *)) { struct aout_data_struct *rawptr, *oldrawptr; - bfd_cleanup cleanup; - size_t amt =3D sizeof (struct aout_data_struct); + bfd_cleanup result; + size_t amt =3D sizeof (*rawptr); =20 rawptr =3D bfd_zalloc (abfd, amt); if (rawptr =3D=3D NULL) - return 0; + return NULL; =20 oldrawptr =3D abfd->tdata.aout_data; abfd->tdata.aout_data =3D rawptr; @@ -549,7 +549,8 @@ NAME (aout, some_aout_object_p) (bfd *abfd, *abfd->tdata.aout_data =3D *oldrawptr; =20 abfd->tdata.aout_data->a.hdr =3D &rawptr->e; - *(abfd->tdata.aout_data->a.hdr) =3D *execp; /* Copy in the internal_exec= struct. */ + /* Copy in the internal_exec struct. */ + *(abfd->tdata.aout_data->a.hdr) =3D *execp; execp =3D abfd->tdata.aout_data->a.hdr; =20 /* Set the file flags. */ @@ -585,7 +586,6 @@ NAME (aout, some_aout_object_p) (bfd *abfd, =20 abfd->start_address =3D execp->a_entry; =20 - obj_aout_symbols (abfd) =3D NULL; abfd->symcount =3D execp->a_syms / sizeof (struct external_nlist); =20 /* The default relocation entry size is that of traditional V7 Unix. */ @@ -599,12 +599,8 @@ NAME (aout, some_aout_object_p) (bfd *abfd, bfd_init_window (&obj_aout_string_window (abfd)); #endif =20 - obj_aout_external_syms (abfd) =3D NULL; - obj_aout_external_strings (abfd) =3D NULL; - obj_aout_sym_hashes (abfd) =3D NULL; - if (! NAME (aout, make_sections) (abfd)) - return NULL; + goto error_ret; =20 obj_datasec (abfd)->size =3D execp->a_data; obj_bsssec (abfd)->size =3D execp->a_bss; @@ -654,9 +650,9 @@ NAME (aout, some_aout_object_p) (bfd *abfd, /* Determine the architecture and machine type of the object file. */ abfd->obj_arch =3D bfd_arch_obscure; =20 - adata(abfd)->page_size =3D TARGET_PAGE_SIZE; - adata(abfd)->segment_size =3D SEGMENT_SIZE; - adata(abfd)->exec_bytes_size =3D EXEC_BYTES_SIZE; + adata (abfd)->page_size =3D TARGET_PAGE_SIZE; + adata (abfd)->segment_size =3D SEGMENT_SIZE; + adata (abfd)->exec_bytes_size =3D EXEC_BYTES_SIZE; =20 return _bfd_no_cleanup; =20 @@ -670,7 +666,7 @@ NAME (aout, some_aout_object_p) (bfd *abfd, header, should cope with them in this callback as well. */ #endif /* DOCUMENTATION */ =20 - cleanup =3D (*callback_to_real_object_p)(abfd); + result =3D (*callback_to_real_object_p) (abfd); =20 /* Now that the segment addresses have been worked out, take a better guess at whether the file is executable. If the entry point @@ -685,7 +681,7 @@ NAME (aout, some_aout_object_p) (bfd *abfd, =20 To fix this, we now accept any non-zero entry point as an indication = of executability. This will work most of the time, since only the linker - sets the entry point, and that is likely to be non-zero for most syst= ems. */ + sets the entry point, and that is likely to be non-zero for most syst= ems. */ =20 if (execp->a_entry !=3D 0 || (execp->a_entry >=3D obj_textsec (abfd)->vma @@ -708,18 +704,19 @@ NAME (aout, some_aout_object_p) (bfd *abfd, issue. Many kernels are loaded at non standard addresses. */ if (abfd->iostream !=3D NULL && (abfd->flags & BFD_IN_MEMORY) =3D=3D 0 - && (fstat(fileno((FILE *) (abfd->iostream)), &stat_buf) =3D=3D 0) + && (fstat (fileno ((FILE *) (abfd->iostream)), &stat_buf) =3D=3D 0) && ((stat_buf.st_mode & 0111) !=3D 0)) abfd->flags |=3D EXEC_P; } #endif /* STAT_FOR_EXEC */ =20 - if (!cleanup) - { - free (rawptr); - abfd->tdata.aout_data =3D oldrawptr; - } - return cleanup; + if (result) + return result; + + error_ret: + bfd_release (abfd, rawptr); + abfd->tdata.aout_data =3D oldrawptr; + return NULL; } =20 /* Initialize ABFD for use with a.out files. */ @@ -1279,38 +1276,43 @@ NAME (aout, set_section_contents) (bfd *abfd, static bool aout_get_external_symbols (bfd *abfd) { +#ifdef USE_MMAP + ufile_ptr filesize =3D bfd_get_file_size (abfd); +#endif + if (obj_aout_external_syms (abfd) =3D=3D NULL) { bfd_size_type count; - struct external_nlist *syms; + struct external_nlist *syms =3D NULL; + bfd_size_type amt =3D exec_hdr (abfd)->a_syms; =20 - count =3D exec_hdr (abfd)->a_syms / EXTERNAL_NLIST_SIZE; + count =3D amt / EXTERNAL_NLIST_SIZE; =20 /* PR 17512: file: 011f5a08. */ if (count =3D=3D 0) - { - obj_aout_external_syms (abfd) =3D NULL; - obj_aout_external_sym_count (abfd) =3D count; - return true; - } + return true; =20 #ifdef USE_MMAP - if (! bfd_get_file_window (abfd, obj_sym_filepos (abfd), - exec_hdr (abfd)->a_syms, - &obj_aout_sym_window (abfd), true)) - return false; - syms =3D (struct external_nlist *) obj_aout_sym_window (abfd).data; + if (filesize >=3D (ufile_ptr) obj_sym_filepos (abfd) + && filesize - obj_sym_filepos (abfd) >=3D amt) + { + if (! bfd_get_file_window (abfd, obj_sym_filepos (abfd), amt, + &obj_aout_sym_window (abfd), true)) + return false; + syms =3D (struct external_nlist *) obj_aout_sym_window (abfd).data; + } + else #else - /* We allocate using malloc to make the values easy to free - later on. If we put them on the objalloc it might not be - possible to free them. */ - if (bfd_seek (abfd, obj_sym_filepos (abfd), SEEK_SET) !=3D 0) - return false; - syms =3D (struct external_nlist *) - _bfd_malloc_and_read (abfd, count * EXTERNAL_NLIST_SIZE, - count * EXTERNAL_NLIST_SIZE); - if (syms =3D=3D NULL) - return false; + { + /* We allocate using malloc to make the values easy to free + later on. If we put them on the objalloc it might not be + possible to free them. */ + if (bfd_seek (abfd, obj_sym_filepos (abfd), SEEK_SET) !=3D 0) + return false; + syms =3D (struct external_nlist *) _bfd_malloc_and_read (abfd, amt, amt= ); + if (syms =3D=3D NULL) + return false; + } #endif =20 obj_aout_external_syms (abfd) =3D syms; @@ -1332,7 +1334,7 @@ aout_get_external_symbols (bfd *abfd) stringsize =3D H_GET_32 (abfd, string_chars); if (stringsize =3D=3D 0) stringsize =3D 1; - else if (stringsize < BYTES_IN_LONG + else if (stringsize + 1 < BYTES_IN_LONG + 1 || (size_t) stringsize !=3D stringsize) { bfd_set_error (bfd_error_bad_value); @@ -1340,7 +1342,9 @@ aout_get_external_symbols (bfd *abfd) } =20 #ifdef USE_MMAP - if (stringsize >=3D BYTES_IN_LONG) + if (stringsize >=3D BYTES_IN_LONG + && filesize >=3D (ufile_ptr) obj_str_filepos (abfd) + && filesize - obj_str_filepos (abfd) >=3D stringsize + 1) { if (! bfd_get_file_window (abfd, obj_str_filepos (abfd), stringsize + 1, &obj_aout_string_window (abfd), true))