I reviewed gcab version 0.6-1 as checked into xenial. This shouldn't be
considered a full security audit but rather a quick gauge of
maintainability.
- gcab provides a library and program to access Microsoft Cabinet files
- Build-Depends: debhelper, dh-autoreconf, gobject-introspection,
gtk-doc-tools, libgirepository1.0-dev, libglib2.0-dev, valac
- Does not itself use networking or cryptography
- Does not itself daemonize
- No pre/post inst/rm
- No initscripts
- No dbus services
- No setuid files
- Provides /usr/bin/gcab executable
- No sudo
- No udev rules
- Runs a test suite at build, very small
- No cronjobs
- Build logs have warnings about bad function casts and signed integer
overflows
- No subprocesses spawned
- Memory management mostly looked sane though zalloc() contains an integer
overflow bug
- Files written to extensively, under control of cabinet files; it looks
like this program is only prepared to pack and unpack files and
directories. The program does try to keep files from referring to
outside the unpack directory but the glib abstraction layer makes this
code very difficult to reason about.
- Logging looked careful
- Environment variable GCAB_DEBUG turns on additional debugging levels
- No privileged operations; I didn't find any calls that could create
non-file or non-directory outputs.
- No cryptography
- No networking
- No privileged portions of code
- No temp file handling
- No WebKit
- Clean cppcheck
- Clean shellcheck
- No PolicyKit
This codebase is complicated; it feels like half the program exists to
manage the glib abstractions layer, I think this program doesn't
sufficiently benefit from the abstractions to justify the extra
complications. That said, it otherwise appeared to programmed carefully,
and the file format is simple enough that it's possible to test simple
permutations of common archive format attacks by hand directly on an
archive. Some hand-testing and a few hours with AFL fuzzing didn't
discover anything CVE-worthy, though there are some caveats:
- Creating the archive assumes input is entirely trusted -- recursive
symlinks are a quick way to consume a lot of disk space. Solving this
would probably require significant work.
- Extracting the archive assumes the destination is entirely trusted.
Symlinks will be followed. (This is usually desirable, but it felt worth
documenting all the same.)
I found an integer overflow bug that may require a CVE depending upon
what parameters may be supplied to it in the zlib library. We should
distro-patch the g_malloc_n() fix until upstream has responded. (This
change is low-risk and shouldn't cause other issues.)
- ./libgcab/cabinet.c zalloc() integer overflow, should use g_malloc_n()
I found several memory leaks on error conditions; these are unlikely to
have security consequences except under the most contrived of situations:
- ./libgcab/cabinet.c cheader_read() memory leak ch->reserved if any RN()
or RS() macros fail
- ./libgcab/cabinet.c cfolder_read() memory leak cf->reserved if RN()
macro fails
- ./libgcab/cabinet.c cdata_read() memory leak cd->reserved if any of RN()
macro, LZXfdi_init(), LZXfdi_decomp(), 'CK' check, inflateInit2(),
inflate(), inflateReset(), inflateSetDictionary() fail
Security team ACK on promoting gcab to main AFTER the zalloc() function
is modified to call g_malloc_n() instead of g_malloc().
Thanks
** Changed in: gcab (Ubuntu)
Assignee: Ubuntu Security Team (ubuntu-security) => (unassigned)
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1475021
Title:
[MIR] gcab
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/appstream-glib/+bug/1475021/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
