Bug 203646 - makefs: Coverity CID 977470: Writes slightly wrong El Torito Boot Record
Summary: makefs: Coverity CID 977470: Writes slightly wrong El Torito Boot Record
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs mailing list
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-10-08 18:51 UTC by scdbackup
Modified: 2019-01-26 15:01 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description scdbackup 2015-10-08 18:51:13 UTC
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);
Comment 1 scdbackup 2015-10-19 21:17:22 UTC
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;
Comment 2 Enji Cooper freebsd_committer 2017-11-05 20:59:21 UTC
Handing a number of makefs, mtree, and msdosfs bugs in my queue over to emaste@.
Comment 3 Ed Maste freebsd_committer 2018-05-28 20:36:31 UTC
Reset assignee - I am not currently looking at this PR.