[MIR] pv

[Julian Andres Klode]

[Availability]
Available in universe, built on all architectures.

[Rationale]
pv shows progress reports for data passing through a pipeline. It has recently become a requirement for GKE and thus needs to be in main.

[Security]
It only passes data from stdin to stdout and writes progress on stderr, so should not be problematic. There do not seem to be any CVEs

[Quality assurance]
Upstream has a test suite run at build. There do not seem to be any important bugs. It just works after installing.

[Dependencies]
Nothing special, just debhelper for build and Depends: libc6 (>= 2.15).

[Standards compliance]

[Maintenance]
foundations-bugs is subscribed now. The package is relatively small, and should not be a huge burden, we can probably keep it synced.

[Background information]

[Mathieu Trudel-Lapierre (cyphermox)]

pv is small and straightforward. Its own unit tests are run as part of the build, and has a bug subscriber. However, it does appear to use IPC. I've reviewed as much of it as I could and it looks like the code is prudent, taking extra steps to check for errors, etc. Still, I think it would be better if the IPC bits were reviewed by the Security Team.

[Seth Arnold (seth-arnold)]

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

[Mathieu Trudel-Lapierre (cyphermox)]

MIR approved.

[Łukasz Zemczak (sil2100)]

$ change-override -c main -S pv
Override component to main
pv 1.6.6-1 in bionic: universe/utils -> main
pv 1.6.6-1 in bionic amd64: universe/utils/optional/100% -> main
pv 1.6.6-1 in bionic arm64: universe/utils/optional/100% -> main
pv 1.6.6-1 in bionic armhf: universe/utils/optional/100% -> main
pv 1.6.6-1 in bionic i386: universe/utils/optional/100% -> main
pv 1.6.6-1 in bionic ppc64el: universe/utils/optional/100% -> main
pv 1.6.6-1 in bionic s390x: universe/utils/optional/100% -> main
Override [y|N]? y
7 publications overridden.