https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=203645
Bug ID: 203645
Summary: makefs: Coverity CID 976312: SIGSEGV with option -l 3
Product: Base System
Version: 11.0-CURRENT
Hardware: Any
OS: Any
Status: New
Severity: Affects Some People
Priority: ---
Component: bin
Assignee: freebsd-bugs@FreeBSD.org
Reporter: scdbac...@gmx.net
usr.sbin/makefs/cd9660.c
CID 976312: Explicit null dereferenced (FORWARD_NULL)i
4. var_deref_op: Dereferencing null pointer conversion_function.
1740 return (*conversion_function)(oldname, newname, is_file);
--------------- Source analysis:
The function pointer is non-NULL only if diskStructure.isoLevel is
1 or 2. A snippet in cd9660_parse_opts() indicates that the value
can indeed be 3:
option_t cd9660_options[] = {
{ "l", &diskStructure.isoLevel, 1, 3, "ISO Level" },
{ "isolevel", &diskStructure.isoLevel, 1, 3, "ISO Level" },
...
The line 1740 is in function cd9660_convert_filename().
ISO 9660 level 3 allows the same file names as level 2.
The use of ISO level 3 is not announced anywhere in the ISO
image but rather becomes visible only if a file is large enough
to need more than one extent. Extents can have 4 GiB - 1 byte
of size.
The offer of ISO level 3 is questionable because neither FreeBSD
nor NetBSD can read files with multiple extents from ISO 9660.
http://lists.freebsd.org/pipermail/freebsd-hackers/2012-April/038552.html
Surely nobody has ever created a level 3 ISO with makefs.
Not only would it SIGSEGV, but also the struct stat.st_size value
is assigned to int64_t cd9660node.fileDataLength without any check in
cd9660.c line 871:
newnode->fileDataLength = node->inode->st.st_size;
Later it is handed over to cd9660_bothendian_dword()
which expects uint32_t (see cd9660/cd9660_conversion.c)
cd9660.c line 834:
cd9660_bothendian_dword(newnode->fileDataLength,
newnode->isoDirRecord->size);
So only one extent will be produced with a size as given by the
low 4 bytes of the disk file size.
--------------- Remedy proposal:
Restrict isoLevel to 1 and 2.
- { "l", &diskStructure.isoLevel, 1, 3, "ISO Level" },
- { "isolevel", &diskStructure.isoLevel, 1, 3, "ISO Level" },
+ { "l", &diskStructure.isoLevel, 1, 2, "ISO Level" },
+ { "isolevel", &diskStructure.isoLevel, 1, 2, "ISO Level" },
Restrict data file size to 4 GiB - 1 byte.
/* Set the size */
- if (!(S_ISDIR(node->type)))
+ if (!(S_ISDIR(node->type))) {
+ if (node->inode->st.st_size > (off_t) 4096 * 1024 * 1024 - 1) {
+
+ >>> error out in appropriate way <<<
+
+ }
newnode->fileDataLength = node->inode->st.st_size;
+ }
To appease checkers use the same conversion function for all
levels above 1:
- else if (diskStructure.isoLevel == 2)
+ else
conversion_function = &cd9660_level2_convert_filename;
--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
freebsd-bugs@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"
