Comment 2 for bug 1746891

Revision history for this message
Seth Arnold (seth-arnold) wrote :

I reviewed pv version 1.6.6-1 as checked into bionic. This isn't a full
security audit but rather a quick gauge of maintainability.

- pv provides a progress display for pipes and a process debugging aid
- There are no CVEs for pv in our database

- Build-Depends: debhelper
- No cryptography
- No networking (though of course file descriptors could be TCP sockets)
- No dependencies
- Does not daemonize
- No pre/post inst/rm scripts
- No initscripts
- No systemd unit files
- No dbus services
- No setuid files
- binary pv in PATH
- No sudo fragments
- No udev rules

- Does not execute subprocesses
- Memory management looked careful -- string handling is more complicated
  than usual, but the cases I inspected closely looked safe
- Some file IO for lock files, reading /proc/pid/fd/ status reports
- Extensive error handling with good logging
- Uses TMPDIR TMP DEBUG LANGUAGE LANG LC_ALL LC_MESSAGES environment
  variables
- No privileged operations
- No cryptography
- No networking
- No temporary files
- No WebKit
- No JavaScript
- No PolicyKit
- Several errors reported by cppecheck, I think they're all false
  positives
- Dozens of warnings in the build due to unchecked write() calls -- all
  are directed to the terminal or to stderr. The write() calls that
  *matter* to the correctness of the program look properly handled.
  The unchecked return value from fscanf() may represent a real bug.
  The unchecked snprintf() truncation may represent a real bug.

The pv sources are defensive in some places and less so in others:
anything relating to the safety of the data appears to be very defensive,
but the code relating to the status display is less so. This is probably
the better decision, as a temporary terminal issue may be less important
than the data transfer that is taking place through the pv tool. This
is clearly a decision the authors have made intentionally as they are
otherwise consistent in handling errors.

Security team ACK for promoting pv to main.

Thanks