[MIR] libdeflate

Bug #1908502 reported by Matthias Klose
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
libdeflate (Ubuntu)
Fix Released
Undecided
Unassigned
Hirsute
Fix Released
Undecided
Unassigned

Bug Description

* Availability

In sync with Debian, built on all architectures
https://launchpad.net/ubuntu/+source/libdeflate/1.7-1

* Rationale

It's a new depends of libtiff

* Security

There is no known security issues

https://security-tracker.debian.org/tracker/source-package/libdeflate
https://people.canonical.com/~ubuntu-security/cve/pkg/libdeflate.html
https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=libdeflate

* Quality assurance

The desktop team is going to subscribe to the package

https://launchpad.net/ubuntu/+source/libdeflate/+bugs
https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=libdeflate

No open reports

The package enables upstream tests during the build and ships basic autopkgtests

* Dependencies

No universe binary dependencies

* Standards compliance

current 4.5.1 standards-version, debhelper compat 13, dh style simple rules

* Maintenance

Upstream is active, the package is maintained in Debian and in sync for Ubuntu

Matthias Klose (doko)
tags: added: rls-hh-incoming
Changed in libdeflate (Ubuntu):
status: New → Incomplete
Changed in tiff (Ubuntu):
status: New → Confirmed
Changed in tiff (Ubuntu):
assignee: nobody → Olivier Tilloy (osomon)
tags: added: update-excuse
Iain Lane (laney)
tags: removed: rls-hh-incoming
description: updated
Changed in tiff (Ubuntu Hirsute):
assignee: Olivier Tilloy (osomon) → nobody
status: Confirmed → Invalid
summary: - [MIR] libtiff5 dependency on libdeflate0
+ [MIR] libdeflate
Changed in libdeflate (Ubuntu Hirsute):
status: Incomplete → New
description: updated
Revision history for this message
Sebastien Bacher (seb128) wrote :

Settings as triaged for libtiff so it's picked up by the proposed migration report

Changed in tiff (Ubuntu Hirsute):
status: Invalid → Triaged
Changed in libdeflate (Ubuntu Hirsute):
assignee: nobody → Sebastien Bacher (seb128)
Changed in tiff (Ubuntu Hirsute):
assignee: nobody → Sebastien Bacher (seb128)
Changed in libdeflate (Ubuntu Hirsute):
assignee: Sebastien Bacher (seb128) → nobody
Changed in libdeflate (Ubuntu Hirsute):
assignee: nobody → Didier Roche (didrocks)
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

[Summary]
Some required TODO and checks to be done. If all those are addressed (mostly investigation), it would be a +1 from the MIR team POV.
However, as this library is parsing data, I think a security review would be needed once all the points below are solved.

Notes:
Required TODOs:
- Check if libtiff can use another compresssion library and if so, if there are strong intents to use deflate instead of that one.
- Investigate the patch disabling test issue (more info on the dedicated section below)
Recommended TODOs:
- Analyse the warnings during build situation (see dedicated section as well)

[Duplication]
libdeflate is expected to be a more efficient 9z compression library. I guess there is no way to use our existing compression library in libtiff, but can you ensure it?

[Dependencies]
OK:
- no other Dependencies to MIR due to this
- no -dev/-debug/-doc packages that need exclusion

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking

[Security]
OK:
- history of CVEs does not look concerning
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port
- does not process arbitrary web content
 does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)

Problems:
- does parse data formats for compressing/decompressing them. Will need a security review.

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time
- test suite fails will fail the build upon error.
- does have a simple test that runs as autopkgtest
- The package will have a team bug subscriber
- no translation present, but none needed for this case?
- not a python/go package, no extra constraints to consider in that regard

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking is in place
- d/watch is present and looks ok
- Upstream update history is good
- Debian/Ubuntu update history is good
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far
- no massive Lintian warnings
- d/rules is quite complex but rather clean
- Does not have Built-Using

Problems:
- one distro patch is named "Simplify tests" while its only effect is removing a test. The name is misleading. The comment is also misleading "Don't run m32 tests on x86_64" while it’s removing it for all supported archs, without a rationale why disabling it.

[Upstream red flags]
OK:
- no Errors during the build
- no incautious use of malloc/sprintf (as far as I can check it)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no use of user nobody
- no use of setuid
- no important open bugs (crashers, etc) in Debian or Ubuntu
- no dependency on webkit, qtwebkit, seed or libgoa-*
- not part of the UI for extra checks

Problems:
- quite a few warning during the build "profile count data file not found [-Wmissing-profile]". Those warning could potentially spread to all binaries using this build. Can you check with upstream about those?

Changed in libdeflate (Ubuntu Hirsute):
assignee: Didier Roche (didrocks) → Sebastien Bacher (seb128)
Revision history for this message
Sebastien Bacher (seb128) wrote :

Checking upstream and alternative solutions they don't seem to have any easy way to use instead something already in main, seeing the performance improvement it's probably worth to bring the new option in main despite that fact though.

The confusing patch issue has been reported to Debian now https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=982383

The profile warning has been reported as https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=982384

Changed in tiff (Ubuntu Hirsute):
assignee: Sebastien Bacher (seb128) → nobody
status: Triaged → Invalid
Changed in libdeflate (Ubuntu Hirsute):
assignee: Sebastien Bacher (seb128) → nobody
no longer affects: tiff (Ubuntu Hirsute)
Mathew Hodson (mhodson)
no longer affects: tiff (Ubuntu)
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

[16:42] <cpaelzer> didrocks: it seems to wait for some actions that you identified on the review
[16:42] <cpaelzer> didrocks: should it maybe set to assigned to someone that is supposed to do them
[16:42] <didrocks> yes, I’m folllowing up on that one once the actions from seb are done

Changed in libdeflate (Ubuntu Hirsute):
assignee: nobody → Didier Roche (didrocks)
status: New → Incomplete
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Ok, I would like to see the security review being done while seb is fetching some info on the disabled tests (investigating why they are disabled is a requirement before the MIR to be offically ACK).
The rest has been answered/fullfilled.

Changed in libdeflate (Ubuntu Hirsute):
assignee: Didier Roche (didrocks) → Ubuntu Security Team (ubuntu-security)
status: Incomplete → New
Revision history for this message
Steve Beattie (sbeattie) wrote :

I reviewed libdeflate 1.7-1 as checked into hirsute. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

libdeflate is a compression/decompression library for the Deflate
compression algorithm, along with associated command line tools. It is
written in C and does not provide any other language bindings.

- There does not appear to be any vulnerability history for libdeflate.
- The only odd build dependency is that it includes zlib1g-dev, but it
  appears to use this for test comparisons.
- No pre/post inst/rm scripts.
- No init scripts.
- No systemd units.
- No dbus services.
- No setuid binaries.
- binaries in PATH:
  libdeflate-tools adds libdeflate-g{,un}zip to path.
- No sudo fragments.
- No polkit files.
- No udev rules.
- A set of unit tests that exercise aspects of the library interface
  are included, as well options for running under valgrind plus afl
  fuzzing support. The unit tests are run during the package build.
  There is one autopkgtest, a very simple limited smoke test.
- No cron jobs.
- Build logs:
  - Several "profile count data file not found [-Wmissing-profile]"
    compiler warnings as mentioned in the primary MIR review.

- No apparent processes spawned.
- Memory management is okay.
- For file I/O, the library expects users of the library to handle this.
  The wrapper tools provided look okay, containing file handling in
  a pair of helper functions.
- For logging, as a shared library it does not do any logging itself,
  again relying on calling programs to log. The logging by programs is
  to stderr and looks fine.
- The only environment variable usage is in test situations.
- The only use of privileged functions is by the tools to restore
  permissions/ownership on the newly compressed or uncompressed file.
- No use of cryptography / random number sources (srand() is used for
  test data generation).
- No use of temp files.
- No use of networking.
- No use of WebKit,
- No use of PolicyKit,

- No cppcheck findings, the only coverity issue in non-test code was
  that the return value for posix_fadvise() was not checked in the
  tools, not a severe issue.

Security team ACK for promoting libdeflate to main.

Changed in libdeflate (Ubuntu Hirsute):
status: New → In Progress
assignee: Ubuntu Security Team (ubuntu-security) → nobody
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Reassigning to seb128 then while he is fetching the last required info before promoting it.

Changed in libdeflate (Ubuntu Hirsute):
assignee: nobody → Sebastien Bacher (seb128)
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

 ./change-override -c main -S power-profiles-daemon
Override component to main
power-profiles-daemon 0.1-1~fakesync2 in hirsute: universe/admin -> main
power-profiles-daemon 0.1-1~fakesync2 in hirsute amd64: universe/admin/optional/100% -> main
power-profiles-daemon 0.1-1~fakesync2 in hirsute arm64: universe/admin/optional/100% -> main
power-profiles-daemon 0.1-1~fakesync2 in hirsute armhf: universe/admin/optional/100% -> main
power-profiles-daemon 0.1-1~fakesync2 in hirsute ppc64el: universe/admin/optional/100% -> main
power-profiles-daemon 0.1-1~fakesync2 in hirsute riscv64: universe/admin/optional/100% -> main
power-profiles-daemon 0.1-1~fakesync2 in hirsute s390x: universe/admin/optional/100% -> main
Override [y|N]? y
7 publications overridden.

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

sorry, wrong tab…

Revision history for this message
Sebastien Bacher (seb128) wrote :

since the Debian maintainer isn't responding I did upload a delta to Ubuntu to remove the controversial patch

Changed in libdeflate (Ubuntu Hirsute):
assignee: Sebastien Bacher (seb128) → nobody
status: In Progress → New
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

LGTM, thanks for working on this! Acking the MIR then.

Changed in libdeflate (Ubuntu Hirsute):
status: New → Fix Committed
Revision history for this message
Matthias Klose (doko) wrote :

no bug subscriber yet ...

Changed in libdeflate (Ubuntu Hirsute):
status: Fix Committed → Incomplete
Revision history for this message
Sebastien Bacher (seb128) wrote :

desktop subscribed now

Changed in libdeflate (Ubuntu Hirsute):
status: Incomplete → New
status: New → Fix Committed
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

$ ./change-override -c main -S libdeflate
Override component to main
libdeflate 1.7-1ubuntu1 in hirsute: universe/misc -> main
libdeflate-dev 1.7-1ubuntu1 in hirsute amd64: universe/libdevel/optional/100% -> main
libdeflate-dev 1.7-1ubuntu1 in hirsute arm64: universe/libdevel/optional/100% -> main
libdeflate-dev 1.7-1ubuntu1 in hirsute armhf: universe/libdevel/optional/100% -> main
libdeflate-dev 1.7-1ubuntu1 in hirsute i386: universe/libdevel/optional/100% -> main
libdeflate-dev 1.7-1ubuntu1 in hirsute ppc64el: universe/libdevel/optional/100% -> main
libdeflate-dev 1.7-1ubuntu1 in hirsute riscv64: universe/libdevel/optional/100% -> main
libdeflate-dev 1.7-1ubuntu1 in hirsute s390x: universe/libdevel/optional/100% -> main
libdeflate-tools 1.7-1ubuntu1 in hirsute amd64: universe/libdevel/optional/100% -> main
libdeflate-tools 1.7-1ubuntu1 in hirsute arm64: universe/libdevel/optional/100% -> main
libdeflate-tools 1.7-1ubuntu1 in hirsute armhf: universe/libdevel/optional/100% -> main
libdeflate-tools 1.7-1ubuntu1 in hirsute i386: universe/libdevel/optional/100% -> main
libdeflate-tools 1.7-1ubuntu1 in hirsute ppc64el: universe/libdevel/optional/100% -> main
libdeflate-tools 1.7-1ubuntu1 in hirsute riscv64: universe/libdevel/optional/100% -> main
libdeflate-tools 1.7-1ubuntu1 in hirsute s390x: universe/libdevel/optional/100% -> main
libdeflate0 1.7-1ubuntu1 in hirsute amd64: universe/libs/optional/100% -> main
libdeflate0 1.7-1ubuntu1 in hirsute arm64: universe/libs/optional/100% -> main
libdeflate0 1.7-1ubuntu1 in hirsute armhf: universe/libs/optional/100% -> main
libdeflate0 1.7-1ubuntu1 in hirsute i386: universe/libs/optional/100% -> main
libdeflate0 1.7-1ubuntu1 in hirsute ppc64el: universe/libs/optional/100% -> main
libdeflate0 1.7-1ubuntu1 in hirsute riscv64: universe/libs/optional/100% -> main
libdeflate0 1.7-1ubuntu1 in hirsute s390x: universe/libs/optional/100% -> main
Override [y|N]? y
22 publications overridden.

Changed in libdeflate (Ubuntu Hirsute):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.