[MIR] libwebm (transitive dependency of libheif)[libheif -> aom -> libwebm]

[Vladimir Petko]

[Availability]

- The package libwebm is already in Ubuntu universe.
- The package libwebm does not build for the architectures
  it is designed to work on.
- It currently builds and works for architectures:
  amd64 arm64 armhf i386 ppc64el riscv64
  It currently fails build unit tests for: s390x
  https://launchpadlibrarian.net/635116394/buildlog_ubuntu-lunar-s390x.libwebm_1.0.0.29-1_BUILDING.txt.gz

  Link to package https://launchpad.net/ubuntu/+source/libwebm/

[Rationale]

- The package libwebm will not generally be useful for a large part of
  our user base, but is important/helpful still because it is vendored
  in aom package that we intend to support as a dependency of libheif.
- It would be great and useful to community/processes to have the
  package libwebm in Ubuntu main, but there is no definitive deadline.

[Security]

- Had 6 security issues in the past
  - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-9746
  - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-6548
  - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-6406
  - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-19212
  - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-2464
  - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-1621
  No CVEs open against current version (1.0.0.29-1).
- no `suid` or `sgid` binaries
- no executables in `/sbin` and `/usr/sbin`
- Package does not install services, timers or recurring jobs
- Packages does not open privileged ports (ports < 1024)
- Packages does contain extensions to security-sensitive software:
  the package provides WebM parser which processes untrusted input

[Quality assurance - function/usage]

- The package works well right after install

[Quality assurance - maintenance]

- The package is maintained well in Debian/Ubuntu and has not too many
  and long term critical bugs open
    - Ubuntu https://bugs.launchpad.net/ubuntu/+source/libwebm/+bug
    - Debian https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=libwebm

[Quality assurance - testing]

- The package runs a test suite on build time, if it fails
  it makes the build fail, link to build log
  https://launchpadlibrarian.net/635116394/buildlog_ubuntu-lunar-s390x.libwebm_1.0.0.29-1_BUILDING.txt.gz

- The package does not run an autopkgtest because it is not implemented

[Quality assurance - packaging]

- debian/watch is present and works
- debian/control defines a correct Maintainer field
- This package does not yield massive lintian Warnings, Errors
- Please link to a recent build log of the package
    https://launchpadlibrarian.net/635115306/buildlog_ubuntu-lunar-amd64.libwebm_1.0.0.29-1_BUILDING.txt.gz
- Please attach the full output you have got from
  `lintian --pedantic` as an extra post to this bug.
- Lintian overrides are not present
- This package does not rely on obsolete or about to be demoted packages.
- This package has no python2 or GTK2 dependencies
- The package will not be installed by default

- Packaging and build is easy, link to d/rules:
  https://git.launchpad.net/ubuntu/+source/libwebm/tree/debian/rules
  Note: currently rules list individual test suites to run. Finding them
  by a file name suffix will reduce maintenance effort.

[UI standards]

- Application is not end-user facing (does not need translation)
- End-user applications without desktop file, not needed because it does not
  provide GUI

[Dependencies]

- No further depends or recommends dependencies that are not yet in main
  Note: build time dependencies on libgmock-dev and libgtest-dev are present.

[Standards compliance]

- This package correctly follows FHS and Debian Policy

[Maintenance/Owner]

- Owning Team will be Foundations Team
- Team is not yet, but will subscribe to the package before promotion

- This does not use static builds
- This does not use vendored code
- This package is not rust based
- The package failed built during the most recent test rebuild:
  https://launchpadlibrarian.net/644058422/buildlog_ubuntu-lunar-s390x.libwebm_1.0.0.29-1_BUILDING.txt.gz

[Background information]

The Package description explains the package well
Upstream Name is libwebm
Link to upstream project https://chromium.googlesource.com/webm/libwebm

[Vladimir Petko (vpa1977)]

It seems that the recommended way to use libwebm is to bundle it[1].

Debian provides libwebm package, but I believe would be reluctant to add headers to make it usable as a dependency[2].

Currently following packages in Debian are bundling libwebm: firefox, qt6-webengine, aom, firefox-esr, libvpx, scummvm, qtwebengine-opensource-src, godot, thunderbird, chromium, sludge [3].

Should we decide to keep bundling it, then all of those packages will require security releases for CVEs. Should we decide to use it as a shared library, we are facing potential dependency rebuilds on the each new version.

The library API/implementation have not had major changes recently, but we should not rule out possibility when all of the packages above rely on the different versions of the bundled library and switching to them to the libwebm package will require patches or will introduce unintended bugs.

[1] https://groups.google.com/a/webmproject.org/g/webm-discuss/c/7ztiZTH8xBA/m/ahIbZOIiN3gJ
[2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1030890
[3] https://codesearch.debian.net/search?q=mkvparser&literal=1&page=2&perpkg=1

[Vladimir Petko (vpa1977)]

Note: a quick check of the bundled libwebms showed that different versions are present, in some cases (e.g. godot, qt6) IWYU was applied and the library is stripped down.
QT6 bundles 2 different versions of libwebm.

[Didier Roche-Tolomelli (didrocks)]

Review for Package: libwebm

[Summary]
MIR team ack under the constraints to resolve the below listed
required TODOs and as much as possible having a look at the
recommended TODOs.
This does need a security review.

Also, all dependencies needs to be MIR acked.

Required TODOs:
- It’s not available on s390x due to the test failing on that architecture. Given that we have time to investigate and there is no rationale to skip that arch, which is officially supported, I think we should fix it first before promoting it to main.
- does not have an autopkgtest suite at all. This is a MIR requirement that can’t be skipped or it needs a manual test plan being run as every release. Maybe you can take some of the tests running at build time if they are testing your package against other third parties?
Recommended TODOs:
- The package should get a team bug subscriber before being promoted
- some warnings during the build. I think those could be fixed or marked as false positive:
./sample_muxer_metadata.cc: In function ‘ParseChapters’:
./sample_muxer_metadata.cc:135:5: warning: ‘t.milliseconds’ may be used uninitialized [-Wmaybe-uninitialized]
./sample_muxer_metadata.cc:120:19: note: ‘t.milliseconds’ was declared here
In member function ‘operator<’,
    inlined from ‘operator<’ at ./webvtt/webvttparser.cc:626:6,
    inlined from ‘ParseChapters’ at ./sample_muxer_metadata.cc:135:22:
./webvtt/webvttparser.cc:642:3: warning: ‘t.seconds’ may be used uninitialized [-Wmaybe-uninitialized]
./sample_muxer_metadata.cc: In function ‘ParseChapters’:
./sample_muxer_metadata.cc:120:19: note: ‘t.seconds’ was declared here
In member function ‘operator<’,
    inlined from ‘ParseChapters’ at ./sample_muxer_metadata.cc:135:22:
./webvtt/webvttparser.cc:636:3: warning: ‘t.minutes’ may be used uninitialized [-Wmaybe-uninitialized]
./sample_muxer_metadata.cc: In function ‘ParseChapters’:
./sample_muxer_metadata.cc:120:19: note: ‘t.minutes’ was declared here
./sample_muxer_metadata.cc: In member function ‘Parse’:
./sample_muxer_metadata.cc:289:5: warning: ‘t.milliseconds’ may be used uninitialized [-Wmaybe-uninitialized]
./sample_muxer_metadata.cc:274:19: note: ‘t.milliseconds’ was declared here
In member function ‘operator<’,
    inlined from ‘operator<’ at ./webvtt/webvttparser.cc:626:6,
    inlined from ‘operator>=’ at ./webvtt/webvttparser.cc:652:71,
    inlined from ‘Parse’ at ./sample_muxer_metadata.cc:289:22:
./webvtt/webvttparser.cc:642:3: warning: ‘t.seconds’ may be used uninitialized [-Wmaybe-uninitialized]
./sample_muxer_metadata.cc: In member function ‘Parse’:
./sample_muxer_metadata.cc:274:19: note: ‘t.seconds’ was declared here
In member function ‘operator<’,
    inlined from ‘operator>=’ at ./webvtt/webvttparser.cc:652:71,
    inlined from ‘Parse’ at ./sample_muxer_metadata.cc:289:22:
./webvtt/webvttparser.cc:636:3: warning: ‘t.minutes’ may be used uninitialized [-Wmaybe-uninitialized]
./sample_muxer_metadata.cc: In member function ‘Parse’:
./sample_muxer_metadata.cc:274:19: note: ‘t.minutes’ was declared here

[Duplication]
There is no other package in main providing the same functionality.

[Dependencies]
OK:
- no other Dependencies to MIR due to this
  - aom ...

[Boyuan Yang (hosiet)]

Hi, Debian's recent libwebm packager here. As of version 1.0.0.30-6, I believe the required blockers are all solved, including autopkgtest that covers all release architectures and patch (from libwebm upstream trunk) to support s390x. Let me know if there are anything more needed (and further patches are all welcome).

[Seth Arnold (seth-arnold)]

Thanks Boyuan, this still needs a security review to finish the process, I'll try to get that started.

[Fabian Toepfer (fabiantoepfer)]

I reviewed libwebm 1.0.0.30-6 as checked into mantic. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

libwebm is a library for processing the WebM container structure, an open
media file format designed for the web. The WebM structure is based on the
Matroska container and can contain video streams in VP8/VP9 format and audio
streams in Vorbis or Opus format. The project contains WebM and MKV parsers
and MKV muxer as library as well as auxiliary tools to process media files
and display information. It is developed as part of the Chromium project by Google.

- CVE History
    - No open vulnerabilities
    - CVE-2016-1621 CRITICAL (android)
    - CVE-2016-2464 HIGH (android)
    - CVE-2018-6406 HIGH (heap-based buffer overflow)
    - CVE-2018-6548 CRITICAL (use-after-free)
    - CVE-2018-19212 MEDIUM (abort)
    - CVE-2019-2126 HIGH (double-free)
    - CVE-2019-9371 MEDIUM (improper input validation)
    - CVE-2019-9746 HIGH (NULL pointer dereference, abort)
- Build-Depends
    - cmake,
    - libgmock-dev (>= 1.10),
    - libgtest-dev (>= 1.10),
    - pkg-config,
    - python
- pre/post inst/rm scripts
  - None
- init scripts
  - None
- systemd units
  - None
- dbus services
  - None
- setuid binaries
  - None
- binaries in PATH
  - dumpvtt
  - vttdemux
  - webm2pes
  - webm2ts
  - webm_info
- sudo fragments
  - None
- polkit files
  - None
- udev rules
  - None
- unit tests / autopkgtests
  - unit tests are existing and running during the build
  - autopkgtests are existing and passing for all archs
- cron jobs
  - None
- Build logs
  - some warnings about uninitialized variables (-Wmaybe-uninitialized)

- Processes spawned
  - None
- Memory management
  - None
- File IO
  - The command line utilities take file paths as argument from the user
- Logging
  - Logging is done in the utility programs mostly for unexpected error cases
- Environment variable usage
  - TEST_TMPDIR and LIBWEBM_TEST_DATA_PATH only for tests
- Use of privileged functions
  - None
- Use of cryptography / random number sources etc
  - Use of rand_r to generate uids for Track and Chapter classes
- Use of temp files
  - Creation of temp files only for tests using mkstemp
- Use of networking
  - None
- Use of WebKit
  - None
- Use of PolicyKit
  - None

- Any significant cppcheck results
  - None
- Any significant Coverity results
  - 171 coverity findings of different types, but a lot of them are duplicates or false positives.
- Any significant shellcheck results
  - Only minor findings in build scripts
- Any significant bandit results
  - None
- Any significant govulncheck results
  - None

Source code analysis:

After skimming through the source code, I've the impression that it is well written and formatted, it contains lots of comments, some TODO statements with improvement tasks what indicates its reviewed by more than one person.

The project contains simple fuzzer harnesses for the mkvparser and webm_parser respectively.
I used them to fuzz both components and additionally fuzzed the included command line based utilities.
I fuzzed all targets with recent AFL++ on instrumented builds with ASAN enabled and with the included samp...

[Christian Ehrhardt  (paelzer)]

thank you, this looks good waiting for the other bits of the heif libraries.
In Progress until it shows up as component mismatch.

[Christian Ehrhardt  (paelzer)]

Slight change here - this isn't needed for libaom3.

Due to the good use of non-embedded libs we now have correct dependency tracking.
That shows that only aom-tools would needed it, which isn't pulled in from libheif.

We could promote it, but if you want that you'd need to seed aom-tools (if it is serving a good purpose) in one of the -supported seeds I guess.

A bit more detail in https://bugs.launchpad.net/ubuntu/+source/libheif/+bug/1827442/comments/55

[Lukas Märdian (slyon)]

Thanks for the investigation of the new dependency tracking!

I'll unsubscribe ~foundations-bugs and move it to "In Progress", as the MIR still passed. So it has the correct state, should it be needed by any team in the future.