Comment 3 for bug 2006461

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

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

> A Ruby/Rack web server built for parallelism

- CVE History:
  - 9 past CVEs (not 12)
    - CVE-2019-16770, CVE-2020-5247, CVE-2020-5249, CVE-2020-11076, CVE-2020-11077, CVE-2021-29509, CVE-2021-41136, CVE-2022-23634, and CVE-2022-24790
    - https://github.com/puma/puma/security/advisories
    - most are HTTP protocol implementation errors
  - security related issues in History.md
  - project has a Security Policy and publishes Security Advisories \o/
  - upstream has a history of making security patches for the current and previous major version \o/
  - GitHub issue and PR tracker are well maintained
  - https://github.com/puma/puma/issues/2789 needs to be resolved
  - Puma::MiniSSL is middleware for OpenSSL
    - https://github.com/puma/puma/issues/2188
    - "Puma has not active maintainer for it's Java code."
  - note that https://github.com/puma/puma/issues/2081 is not inherently a security issue
- Build-Depends?
  - lunar main
     - debhelper-compat (debhelper)
     - curl
     - libssl-dev (openssl)
     - rake
     - ruby-minitest-stub-const (ruby)
  - lunar universe
     - gem2deb
     - ruby-nio4r (in MIR)
     - ruby-ffi (in MIR)
     - ruby-localhost
- pre/post inst/rm scripts?
  - none
- init scripts?
  - none
- systemd units?
  - ./lib/puma/systemd.rb
  - initiliazes Puma by triggering Puma::Events to manage Puma:Server
  - SdNotify.watchdog optionally implemented for keep-alive notifications
  - intervable can be set with ENV WATCHDOG_USEC
- dbus services?
  - none
- setuid binaries?
  - none
- binaries in PATH?
  - ./usr/bin/puma
  - ./usr/bin/pumactl
  - ./usr/lib/x86_64-linux-gnu/rubygems-integration/3.1.0/gems/puma-5.6.5/bin/puma
  - ./usr/lib/x86_64-linux-gnu/rubygems-integration/3.1.0/gems/puma-5.6.5/bin/pumactl
- sudo fragments?
  - none
- polkit files?
  - none
- udev rules?
  - none
- unit tests / autopkgtests?
  - contains build tests
    - some tests are, reasonably, skipped for missing optional dependencies
    - two tests skipped for "failing in Ubuntu autopkgtest env"
      - looks o-k from changelog notes
  - arm64 no longer passes autopkgtests as reported in initial MIR
- cron jobs?
  - none
- Build logs:
  - nothing beyond MIR Team's lintian findings

- Processes spawned?
  - JRuby exec ignored for MIR
  - Puma::Launcher's use Kernel.exec() is assumed to have trusted input
    - exec set with Puma::Configuration options or ran for bundler
  - Puma::Rack arguments, --eval and --builder, are also assumed to have trusted input
- Memory management?
  - most memory use is standard Ruby, but Puma's MiniSSL contians C code in ./ext/puma_http11/
  - malloc, free, and memcpy use appear safe
- File IO?
  - quite a bit of Ruby file IO -- appears okay in input is trusted
  - chmod and chown in binder looks safe
  - see temp section below
- Logging?
  - mostly defined in Puma::CommonLogger and Puma::ErrorLogger which other files build on
    - Puma::CommonLogger attempts to use built in logger, but it appears other loggers like Rack::CommonLogger
    - Puma::ErrorLogger appears to connect to stdout and stdio
  - client_error's ConnectionError and EOFERROR are discarded/not logged
  - has some debug settings
  - Ruby and C logging looks sane
- Environment variable usage?
  - fine if input trusted
    - safe assumption as untrusted input likely means attacker controls configs
  - mostly used in server launch
- Use of privileged functions?
  - none
- Use of cryptography / random number sources etc?
  - Puma's MiniSSL is middleware to OpenSSL
    - MiniSSL is written in C and Ruby
    - see discussion on replacing MiniSSL: https://github.com/puma/puma/issues/2188
    - from dev comments, Puma does not want the burden of maintaining MiniSSL and consider it a risky feature to support
    - 60 lines contain "SSL" in History.md--high maintenance cost
  - Security Team recommends against using MiniSSL
- Use of temp files?
  - contains plugin where server is restarted if ./tmp/restart.txt is touched
- Use of networking?
  - heavy use
  - default binding to 0.0.0.0
  - networking files such as Puma::DSL and Puma::Server are well documented with inline comments
  - past CVEs mostly relate to HTTP protocol implementation
- Use of WebKit?
  - none
- Use of PolicyKit?
  - none

- Any significant cppcheck results?
  - none
- Any significant Coverity results?
  - results of puma_parser_finish() not used in HttpParser_finish()
  - tools/Dockerfile runs as the default user, root
    - not a concern for MIR
  - results from a Coverity run in September 2022 have been resolved \o/
- Any significant shellcheck results?
  - not significant
- Any significant bandit results?
  - none
- Any significant rubocop results?
  - evals in lib/puma/rack/builder.rb
    - see processes spawned section above

Security team ACK for promoting puma to main.