usr.sbin/makefs/cd9660/cd9660_eltorito.c CID 977470: Out-of-bounds access (OVERRUN) 2. overrun-buffer-val: Overrunning array diskStructure.boot_descriptor->boot_catalog_pointer of 4 bytes by passing it to a function which accesses it at byte offset 4. 374 cd9660_bothendian_dword(first_sector, 375 diskStructure.boot_descriptor->boot_catalog_pointer); --------------- Source analysis: cd9660_bothendian_dword() indeed writes 8 bytes (both endian) into boot_catalog_pointer. usr.sbin/makefs/cd9660.h defines typedef struct _iso9660_disk { ... boot_volume_descriptor *boot_descriptor; ... } iso9660_disk; usr.sbin/makefs/cd9660/cd9660_eltorito.h defines typedef struct _boot_volume_descriptor { ... u_char boot_catalog_pointer [ISODCL(0x47,0x4A)]; u_char unused2 [ISODCL(0x4B,0x7FF)]; } boot_volume_descriptor; So the overrun hits the first 4 bytes of .unused2 . The little endian 4-byte value gets written to .boot_catalog_pointer, even on big endian architectures. This could be very bad if used for more computations. But obviously this will only be written as byte string to the ISO image. El Torito 1.0 (1995) Figure 7 specifies bytes 0x4B to 0x7FFF of the record as "Unused, must be 0." But FreeBSD-11.0-CURRENT-amd64-20151001-r288459-disc1.iso has at byte address (17 * 2048 + 0x4B) the values {0, 0, 0, 19} which is the big endian address of the boot catalog. --------------- Remedy proposal: Use function cd9660_731() instead of cd9660_bothendian_dword(): - cd9660_bothendian_dword(first_sector, + cd9660_731(first_sector, diskStructure.boot_descriptor->boot_catalog_pointer);
Also present in NetBSD. http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/makefs/cd9660/cd9660_eltorito.c?annotate=1.17.2.3 359: cd9660_bothendian_dword(first_sector, 360: diskStructure->boot_descriptor->boot_catalog_pointer); http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/makefs/cd9660/cd9660_eltorito.h?annotate=1.5 67: u_char boot_catalog_pointer [ISODCL(0x47,0x4A)]; 68: u_char unused2 [ISODCL(0x4B,0x7FF)]; 69: } boot_volume_descriptor;
Handing a number of makefs, mtree, and msdosfs bugs in my queue over to emaste@.
Reset assignee - I am not currently looking at this PR.
https://reviews.freebsd.org/D39231
committed referencing the other makefs PR by accident: commit 9f2a525360473a778f91021e3be58fd4bfd72ee5 (HEAD -> main, freebsd/main) Author: Ed Maste <emaste@FreeBSD.org> Date: Thu Mar 23 13:02:44 2023 -0400 makefs: correct El Torito bood record The boot catalog pointer is a DWord, but we previously populated it via cd9660_bothendian_dword which overwrote four unused bytes following it. See El Torito 1.0 (1995) Figure 7 for details. PR: 203531 Reported by: Coverity Scan Reported by: Thomas Schmitt <scdbackup@gmx.net> Reviewed by: kevans CID: 977470 Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D39231
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=b95746135307c8146e342e55928bc27e1521f035 commit b95746135307c8146e342e55928bc27e1521f035 Author: Ed Maste <emaste@FreeBSD.org> AuthorDate: 2023-03-23 17:02:44 +0000 Commit: Ed Maste <emaste@FreeBSD.org> CommitDate: 2023-04-10 13:37:40 +0000 makefs: correct El Torito bood record The boot catalog pointer is a DWord, but we previously populated it via cd9660_bothendian_dword which overwrote four unused bytes following it. See El Torito 1.0 (1995) Figure 7 for details. PR: 203531, 203646 Reported by: Coverity Scan Reported by: Thomas Schmitt <scdbackup@gmx.net> Reviewed by: kevans CID: 977470 Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D39231 (cherry picked from commit 9f2a525360473a778f91021e3be58fd4bfd72ee5) usr.sbin/makefs/cd9660/cd9660_eltorito.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)