Comment 6 for bug 2004119

I reviewed pappl 1.3.1-2 as checked into Mantic. This shouldn't be considered a
full audit but rather a quick gauge of maintainability.

PAPPL is a library built in C that helps in developing printing applications
for Common UNIX Printing System (CUPS). These are intended to be a replacement
for old printing drivers. The source package produces two different binary
packages, namely libpappl1 and libpappl-dev (for static libraries, headers,
and documentation).

- CVE History:
  - No CVE assigned for PAPPL
  - There were some GitHub issues with security impact (for example, crashes
    mentioned in the issues #61, #121, and #272). But the delay between
    reporting and fixing is one day.
- Build-Depends
  - All libraries are with development files and headers.
  - Their purposes are:
    - Printing
        - libcups2-dev: CUPS library (printer and print job management
          capabilities)
        - libcupsimage2-dev: support for handling images within CUPS
    - Networking
        - libavahi-client-dev: Avahi client library (service discovery on a
          local network)
    - Cryptography
        - libgnutls28-dev: GnuTLS, for implementations for SSL, TLS, and DTLS
          protocols
    - I/O
        - libusb-1.0-0-dev: user-space access to USB devices
    - File formats
        - libpng-dev: operations with PNG images
        - libjpeg-dev: operations with JPEG images
        - zlib1g-dev: zlib compression and decompression
    - IAM
        - libpam0g-dev: authentication tasks involving Unix's PAM
- pre/post inst/rm scripts
  - N/A
- init scripts
  - N/A
- systemd units
  - N/A
- dbus services
  - N/A
- setuid binaries
  - N/A
- binaries in PATH
  - /usr/bin/pappl-makeresheader, for libpappl-dev: Creates a C header file
    suitable for inclusion in a printer application. Based on makeresheader.sh.
    All usages of the parameters (namely the filenames) are escaped with quotes
    (e.g. "/* $file */").
- sudo fragments
  - N/A
- polkit files
  - N/A
- udev rules
  - N/A
- unit tests / autopkgtests
  - TBR
- cron jobs
  - N/A
- Build logs:
  - False positive use-after-free warning in client-webif.c:214: The "body"
    pointer (which is created by strdup-ing another string) cannot be used
    after its free(). If the free() occurs, then the function already returns.
  - False positive spelling-error-in-binary warning from Lintian: already
    documented in libpappl1.lintian-overrides
- Processes spawned
  - Calls to system() in testpappl.c, but with hard-coded values
  - The same for posix_spawn, where no parameter is user-controlled
- Memory management
  - Only a calloc() without checking the return value, but in
    testsuite/pwg-driver.c:608
  - No strcpy. All operations are achieved with memcpy, which implies the
    specification of sizes.
- File IO
  - umask is not used.
  - Double close in printer-webif.c:1133, but the file descriptors are
    function-scoped
- Logging
  - All exposed functions in log.h (which internally are using write_log)
    - Enum value (represented in memory as an int) is not verified. If it's set
    other value, which should be less than a loglevel member from the system
    argument (set during the papplSystemCreate API call), then arbitrary memory
    read happens
      - Fixed in 4c0d022557df8babb08dacd365149192a8820e7e
    - If the string is not NULL terminated (e.g. by mistakingly using char
    string[2] = "ab"), then stack will be accessed (out-of-bound) until
    encountering a NULL.
- Environment variable usage
  - Copies of getenv's results are made with safe string copy operations,
    including buffers' length.
- Use of privileged functions
  - Only a ioctl call to set the printer's status
- Use of cryptography / random number sources etc
  - Strong private keys generated (e.g. RSA 4096 and ECDSA 384)
  - Reading from /dev/urandom for the papplGetRand function, but is used or
    exposed only for creating nounced and UIDs. This aspect is clearly stated
    in the function's documentation.
  - The buffers in which sensitive data is stored (either allocated by OpenSSL
    or GnuTLS) are not zeroed after use. The same applies to cookies and
    password hashes, which are stored in PAPPL's data structures. APIs like
    OPENSSL_cleanse and BN_clear_free can be used to avoid situations in which
    the process memory is read, leaking sensitive information.
  - Timing attacks possible in papplClientHTMLAuthorize, for both cookies
    (client-webif.c:474) and passwords (client-webif.c:509 and
    system-webif.c:1354) due to the usage of a standard string compare
    functions (strcpy, strncpy)
      - Fixed in 3a0bbb2ca00df504cddd1f5541484a6eeeeabf73 and
        5c1acfee2caaf2244c0b54942201d94d8fbb1afb
- Use of temp files
  - No mktemp calls
  - papplCreateTempFile, a function exposed in the API, has a path traversal
    via the extension, which is not sanitized. (CHANGE)
      - An attacker with access to the folder with temporary files (either the
        one pointed in the TMPDIR envvar or /tmp) can observe the filenames
        which are used. The first one is hard to deduce as it's based on
        /dev/urandom's content, but the next ones are computed with the
        Mersenne Twister PRNG.
      - This permits the attacker to predict the next filenames to be chosen,
        to create folders in advance for these filenames, and to use the
        extension "/.." * N + <absolute_target_path>.
      - The impact of this bug depends on how the path is further used by the
        application linking and using PAPPL.
      - Fixed in ed1d3378baa0658f6390818aa202357ec1351325
- Use of networking
  - The majority of networking functions are only proxies, being exposed in
    the library API. The buffering is executed correctly, by considering the
    lengths to not create overflows.
- Use of WebKit
  - N/A
- Use of PolicyKit
  - N/A
- Any significant cppcheck results
  - Resource leak in pappl/parse-lock-log.c:93, when a return is made without
    freeing a file pointer (opening a file argument). There is only a file
    pointer (w/o any recurrence), so no concerns about memory exhaustion.
- Any significant Coverity results
  - Multiple TOCTOUs while creating directories with access + mkdir combo
    - Identified by Coverity in:
      - job.c:462
      - printer.c:887
      - system-webif.c:2329
      - system-webif.c:2329
      - system-webif.c:2340
      - system-webif.c:2557
      - system-webif.c:2568
      - mainloop-subcommands.c:783
      - mainloop-subcommands.c:1773
      - mainloop-subcommands.c:810
      - mainloop-subcommands.c:1798
    - The same situation was encountered in curl's codebase, with a fix already
    available here: https://github.com/curl/curl/commit/eb1592ec84bec8899a6642\
    01a7f2298ace059ea9.
    - Fixed in 6dcc29cb2a803c5715c7812d6925679a54a5ee23
- Any significant shellcheck results
  - Lots of errors, but in development scripts like configure and install-sh
- Any significant bandit results
  - N/A
- Any significant govulncheck results
  - N/A

The codebase is very well maintained and qualitative. The security team will
give the ACK for promoting pappl to main.

Thanks to Michel for having prompt responses, and a great openness to fix the
encountered bugs and update the security policy!