Comment 47 for bug 1636666

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

I reviewed pcre2 version 10.32-3 as checked into disco. This shouldn't
be considered a full security audit, but rather a quick gauge of
maintainability.

- pcre2 is a regular expression library

- There are 25 CVEs for pcre2 in our database -- though this may be an
  over-count or under-count, as the pcre2 project has an awkward
  relationship with fuzzer-found issues:

  - The pcre2 team says any memory-unsafety due to using
    PCRE2_NO_UTF_CHECK (possibly other flags) is entirely programmer
    error. This is almost reasonable enough except it's a shame that a
    brand-new RE library has memory safety issues in this day and age.

  - So some of these CVEs may be disputed / rejected / ignored by
    upstream as a "don't do that" issue. And, likewise, because a great
    number of people have reported a great number of memory-safety issues
    to the pcre2 developers, there's a very real chance that actual
    security issues have *not* been assigned CVEs due to general fatigue.

- Build-Depends: debhelper (>=9), dpkg-dev (>= 1.16.1~)
- Does not daemonize
- No pre/post inst/rm
- No init scripts
- No systemd unit files
- No dbus services
- No setuid
- pcre2grep, pcre2-config, pcre2test in PATH
- No sudo fragments
- No udev rules
- Decent sized test suite, run during the build, that will be helpful

- Subprocesses can be spawned via pcre2grep -- this MUST be disabled in
  our packaging
- Memory management is extremely intricate and involved
- The JIT uses O_TMPFILE files, pcre2grep will automatically ungzip,
  unbz2, or read normal files
- Error logging looked careful
- JIT uses secure_getenv("TMPDIR"), pcre2grep uses some colour and
  locale environment variables
- No privileged operations
- No cryptography
- No privileged portions of code
- Temp file handling looked careful
- No WebKit
- No PolicyKit
- Some cppcheck errors -- some false positives. The others were fixed very
  shortly after I filed a bug: https://bugs.exim.org/show_bug.cgi?id=2355

Please disable SUPPORT_PCRE2GREP_CALLOUT in our builds. Absolutely no
one will expect a tool named 'grep' to execute commands as part of what
it does. (The execution itself looks fine; it is the fact that it *does*
execute that is extremely surprising.)

pcre2 is extremely dense code. Maintenance will require upstream's
assistance for nearly every issue. Operations as common and expensive
as regular expression matching may justify the code complexity but the
large number of memory-safety issues is a strong concern.

However enough applications have converted to pcre2 that I'm not sure we
can realistically hold off supporting pcre2 any longer. The maintenance
costs to keep all supported packages using older pcre will eventually
outweigh the costs to maintain pcre2, so we might as well adapt.

Security team ACK to promote pcre2 to main CONDITIONAL upon disabling
SUPPORT_PCRE2GREP_CALLOUT in our packages.

Thanks