Comment 10 for bug 1978066

Revision history for this message
Mark Esler (eslerm) wrote :

I reviewed jigit 1.22-3ubuntu1 as checked into kinetic. This shouldn't be considered a full audit but rather a quick gauge of maintainability.

jigit is a package of utilities to make working with jigdo files easier. jigdo files break large file, like a DVD ISO, into smaller constituent files.

- no CVE history
- build-depends on debhelper, zlib1g-dev, libbz2-dev, libio-compress-perl
- no pre/post inst/rm scripts, init scripts, systemd units, dbus services, setuid binaries, sudo fragments, polkit files, udev rules, cron jobs, WebKit, PolicyKit, nor privileged functions.
- binaries:
  - /usr/bin/jigdo-gen-checksum-list
  - /usr/bin/jigdump
  - /usr/bin/jigit-mkimage
  - /usr/bin/jigsum
  - /usr/bin/jigsum-sha256
  - /usr/bin/parallel-sums
  - /usr/sbin/mkjigsnap
- autopkgtest proposed https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1012621
- Build logs:
  - 3x "warning: ignoring return value of ‘fread’" in mkimage.c and uncompress.c
- Processes spawned:
  - in ./iso-image.pl:89, ./iso-image.pl:110, ./libjte/jte.c:189, ./libjte/jte.c:237, ./libjte/checksum.c:679
  - filename path in checksum.c main() is not checked or sanitized and can buffer overflow before system(buf)
- Memory management / File IO / Logging
  - many files manage memory, IO, and logging
  - user defined file paths are not sanitized (e.g., mkimage.c)
  - memory is not being freed
  - see ccpcheck and coverity results
- Environment variable usage
  - only in build files
- Use of cryptography:
  - no -- only checksums
- Use of temp files:
  - no -- only in build scripts and documentation/comments
- Use of networking?
  - only in, non-installed, helper script `./jigit`
- Significant cppcheck results:
  - extract-data.c:46:9: error: Resource leak: in_file [resourceLeak]
  - libjte/jte.c:649:9: error: Memory leak: checksum [memleak]
  - libjte/test/demo.c:213:9: error: Resource leak: fp_out [resourceLeak]
  - uncompress.c:157:9: error: Memory leak: comp_buf [memleak]
  - uncompress.c:165:9: error: Memory leak: comp_buf [memleak]
  - mkimage.c:1258:7: error: Used file that is not opened. [useClosedFile]
  - mkimage.c:1013:5: error: Common realloc mistake: 'buf' nulled but not freed upon failure [memleakOnRealloc]
- Significant code-c.txt:
  - unsafe input mechanisms ./mkimage.c:(602|620|630): ret = gzgets(file, buf, sizeof(buf));
- Significant Coverity results:
  - many Resource Leak (CWE-404) reports (mostly leaked storage)
  - many Uninitialized Scalar Variable (CWE-457) reports
  - many Unchecked return value from library (CWE-252) reports
  - other reports not mentioned here
- Significant shellcheck results:
  - helper shell script `./jigit` feels dubious, but does not install
    - defaults to download config from private repository
    - many ShellCheck warnings
  - libjte/bin/jigdo-gen-checksum-list:139:17: warning: For loops over find output are fragile. Use find -exec or a while read loop. [SC2044]
- Other
  - Maintainer is actively involved in FOSS and Debian \o/
  - Most code written ~17 and ~11.5 years ago
  - several "TODO" comments in libjte.c and checksum.c
  - parse_data_block() is defined in mkimage.c and jigdump.c
  - read_data_block() is defined in uncompress.c and jigdo.c (no aparent relation)

jigit is a useful set of tools for distributing large files however I don't think jigit is sufficiently paranoid for wide use. jigit should only be ran on controlled environments on controlled, known safe, input.

Security team NACK for promoting jigit to main.