Bug #2004449 “[MIR] libde265 (dependency of libheif)” : Bugs : libde265 package : Ubuntu

Revision history for this message

Vladimir Petko (vpa1977) wrote on 2023-02-01:

#1

libde265-lintian.txt Edit (338 bytes, text/plain)

Revision history for this message

Lukas Märdian (slyon) wrote on 2023-02-01 (last edit on 2023-02-01):

#2

We should probably exclude the `libde265-examples` binary package from promotion, to avoid the additional (big) MIRs for Qt, SDL & ffmpeg

Also, we should probably try to get some basic tests implemented and security issues fixed to mke it fit for promotion.

summary:

- [MIR] libde265
+ [MIR] libde265 (dependency of libheif)

Christian Ehrhardt  (paelzer) on 2023-02-07

Changed in libde265 (Ubuntu):
assignee:	nobody → Christian Ehrhardt  (paelzer)

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2023-02-09 (last edit on 2023-02-09):

#3

Download full text (6.0 KiB)

Review for Package: libde265

[Summary]
MIR team ACK under the constraint to resolve the below listed
required TODOs and as much as possible having a look at the
recommended TODOs. This is a bit closer than usual, but since
we have time you can work with upstream on those issues.

This does need a security review, so I'll assign ubuntu-security.
@security - please have a look at the past CVEs, it seemed to me some were
handled slowly, many are still open is that a concern?

List of specific binary packages to be promoted to main: libde265-0, libde265-dev
Specific binary packages built, but NOT to be promoted to main: libde265-examples

Required TODOs:
#1 - Strictly speaking de256 has an encoding functionality, if there would be a
  chance could we use it for both and ignore x265 (or vice versa)?
  Could someone please engage with upstream if that is possible/feasible
  and either modify it to work that way or state and refer to the reasoning
  why it can't work well here?
  Also more common libs like ffmped/libavcodec do this, but they are not in main either :-/
#2 - this lacks build and autopkgtests, please add both to add some to make
  this properly covered and spot issues early on.
#3 - upstream releases are fine, but sporadic. Please confirm that you are ok
  to cover work that might happen due to another downtime of year(s) and that
  you are confident and comfortable to own this package despite that.

Recommended TODOs:
#4 - Lintian warnings https://udd.debian.org/lintian/?packages=libde265
Could be worth a look to improve on them

[Duplication]
There seems to be no other package in main yet, but one has to wonder x265
AND de265 isn't that too much and duplicate at the current request level?
But while I'd wish we could pick just one, it seems one is better at decoding
and the other at encoding.
From https://github.com/strukturag/libheif:
"libheif makes use of libde265 for HEIF image decoding and x265 for encoding."
de265 was added first, but the git history isn't mentioning in detail why
x265 was added on top other than "for encoding".

While I'd wish we could pick just one, it seems that isn't working well.
I'll add a todo to at least try this and maybe disuss it upstream.
=> There is no other package in main providing the same functionality (kind of).

[Dependencies]
OK:
- libheif will depend on libde265-0 which has no further dependencies outside
  of main
  -> no other Dependencies to MIR due to this
- no -dev/-debug/-doc packages that need exclusion (libde265-dev has just
  libde265-0 as dependency)
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems: None

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking
- does not have unexpected Built-Using entries
- not a go package, no extra constraints to consider in that regard
- not a rust package, no extra constraints to consider in that regard

Problems: None

[Security]
OK:
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port/socket
- does not process arbitrary web content
- does not use centralized online accounts
- does not integrate arb...

Review for Package: libde265

[Summary]
MIR team ACK under the constraint to resolve the below listed
required TODOs and as much as possible having a look at the
recommended TODOs. This is a bit closer than usual, but since
we have time you can work with upstream on those issues.

This does need a security review, so I'll assign ubuntu-security.
@security - please have a look at the past CVEs, it seemed to me some were
handled slowly, many are still open is that a concern?

List of specific binary packages to be promoted to main: libde265-0, libde265-dev
Specific binary packages built, but NOT to be promoted to main: libde265-examples

Required TODOs:
#1 - Strictly speaking de256 has an encoding functionality, if there would be a
  chance could we use it for both and ignore x265 (or vice versa)?
  Could someone please engage with upstream if that is possible/feasible
  and either modify it to work that way or state and refer to the reasoning
  why it can't work well here?
  Also more common libs like ffmped/libavcodec do this, but they are not in main either :-/
#2 - this lacks build and autopkgtests, please add both to add some to make
  this properly covered and spot issues early on.
#3 - upstream releases are fine, but sporadic. Please confirm that you are ok
  to cover work that might happen due to another downtime of year(s) and that
  you are confident and comfortable to own this package despite that.

Recommended TODOs:
#4 - Lintian warnings https://udd.debian.org/lintian/?packages=libde265
  Could be worth a look to improve on them

[Duplication]
There seems to be no other package in main yet, but one has to wonder x265
AND de265 isn't that too much and duplicate at the current request level?
But while I'd wish we could pick just one, it seems one is better at decoding
and the other at encoding.
From https://github.com/strukturag/libheif:
"libheif makes use of libde265 for HEIF image decoding and x265 for encoding."
de265 was added first, but the git history isn't mentioning in detail why
x265 was added on top other than "for encoding".

While I'd wish we could pick just one, it seems that isn't working well.
I'll add a todo to at least try this and maybe disuss it upstream.
=> There is no other package in main providing the same functionality (kind of).

[Dependencies]
OK:
- libheif will depend on libde265-0 which has no further dependencies outside
  of main
  -> no other Dependencies to MIR due to this
- no -dev/-debug/-doc packages that need exclusion (libde265-dev has just
  libde265-0 as dependency)
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems: None

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking
- does not have unexpected Built-Using entries
- not a go package, no extra constraints to consider in that regard
- not a rust package, no extra constraints to consider in that regard

Problems: None

[Security]
OK:
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port/socket
- 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)
- does not deal with security attestation (secure boot, tpm, signatures)
- does not deal with cryptography (en-/decryption, certificates, signing, ...)

Problems:
- does parse data formats (images) from untrusted sources
  And that has been an attack vector in the past.
- history of CVEs does look concerning, there was just one CVE but it took
  ~2 years to close it and even then the references weren't updated.
  Furthermore there seem to be just "30 CVEs" fixed in the last version. Might
  be a coincidence, but that is much.
  And there seem to be 35 more open except in the most recent version
  according to
  https://security-tracker.debian.org/tracker/source-package/libde265
  This could be just a red herring with one underlying cause responsible
  for all these issues, but I'd like security to have a look anyway, they will
  know better.

I'd leave the rating of that to the security team, it needs their review anyway.

[Common blockers]
OK:
- does not FTBFS currently
- This does not need special HW for build or test
- no new python2 dependency

Problems:
- does not have a test suite that runs at build time
- does not have a non-trivial test suite that runs as autopkgtest

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking is in place it has a few wanrings but on explcitly
  optional symbols
- d/watch is present and looks ok
- Debian/Ubuntu update history is good (matches upstream, and there isn't much
  more one could do)
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far
  maintained the package
- d/rules is rather clean
- It is not on the lto-disabled list

Problems:
- Upstream update history is sporadic, there are spikes of 1-4 releases
  in a short time and then nothing for years (like nov 20 -> oct 22).
  That isn't super-bad but indicates that is isn't the most organized
  huge project with regular releases.
- Lintian warnings
  https://udd.debian.org/lintian/?packages=libde265
  Could be worth a look to improve on them

[Upstream red flags]
OK:
- no Errors/warnings during the build (just a few deprecations)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside
  tests)
- no use of user nobody
- no use of setuid
- use of setuid, but ok because TBD (prefer systemd to set those
  for services)
- 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
- no translation present, but none needed for this case (not user visible)

Problems:
- usage of allocations, there are a bunch of open "not reproducible" issues
  in regard to allocation, but also issues like
  https://github.com/strukturag/libde265/issues/330
  Individually not super-bad, but together with the CVEs not too great either.

Changed in libde265 (Ubuntu):
assignee:	Christian Ehrhardt  (paelzer) → Ubuntu Security Team (ubuntu-security)

Seth Arnold (seth-arnold) on 2023-02-11

tags:

added: sec-1688

Revision history for this message

Vladimir Petko (vpa1977) wrote on 2023-04-05:

#4

Duplication:
As per upstream[1] the encoding functionality of libde265 is limited and not intended for production use.

[1] https://github.com/strukturag/libheif/issues/797#event-8800456603

Revision history for this message

David Fernandez Gonzalez (litios) wrote on 2023-09-07:

#5

Download full text (4.4 KiB)

I reviewed libde265 1.0.12-1build1 as checked into mantic. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

libde265 is an open-source implementation of the h.265 video codec. It provides
both an encoding and decoding API but the main objective of the library is to
provide h.265 decoding, as the encoding part is not actively developed [1]

This package was requested due to it being a dependency of libheif.
Note that libheif is developed by the same developer as libde265.

- CVE History
  - 48 CVEs since 2020.
  - The developer started to commit seriously to patching CVEs recently
    * Several security-only releases in the past months.
    * One of them patched 30 CVEs.
    * Reference to specific commits patching the CVE.
  - Issues were not addressed on time but recent upstream activity shows
    that the developer is now actively answering and fixing them.
- Build-Depends
  - All libraries are related to image processing, nothing out of normal.
- pre/post inst/rm scripts
  - None
- init scripts
  - None
- systemd units
  - None
- dbus services
  - None
- setuid binaries
  - None
- binaries in PATH
  - from the ".*examples" binary:
    - libde265-sherlock265: QT video player.
    - libde265-dec265: simple player for h.265 bitstreams.
- sudo fragments
  - None
- polkit files
  - None
- udev rules
  - None
- unit tests / autopkgtests
  - No unit tests available in the software.
  - No autopkgtests.
  - CI/CD in GitHub uses example input files to test (x86 and arm) as well as Valgrind + Coverity analysis.
- cron jobs
  - None
- Build logs
  - Some macro warnings.
  - Some libtool reminder warnings.
  - Some warnings related to exceeding maximum object size, depending on the input file.
- Processes spawned
  - No process spawned in the library, only on the tools and the examples.
- Memory management
  - The code performs checks to ensure malloc doesn't fail.
  - The code doesn't check the boundaries when calling memcpy (on internal functions only).
    Hard to trace if everything is checked before but many of the CVEs are because of assuming the right inputs.
  - Sprintfs are handled safely by using limits on the size.
- File IO
  - Library reads H.265/HEVC files to decode/encode them.
  - The API functions don't sanitize paths but it's a library so it should be checked by the frontend application.
  - No umask usage
- Logging
  - Done safely through local specific functions.
  - No possibility of format string.
- Environment variable usage
  - None
- Use of privileged functions
  - None
- Use of cryptography / random number sources etc
  - None
- Use of temp files
  - Only on the tools, not on the library.
- Use of networking
  - None
- Use of WebKit
  - None
- Use of PolicyKit
  - None

- Any significant cppcheck results
  - Mainly non-serious issues related to the encoder.
  - One issue related to memory management, no check in case of an error that will result in BOF.
- Any significant Coverity results
  - Nothing significant.
- Any significant shellcheck results
  - None
- Any significant bandit results
  - No Python code.
- Any significant govulncheck results
  - No Go code.
- Any significant Semgre...

I reviewed libde265 1.0.12-1build1 as checked into mantic. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

libde265 is an open-source implementation of the h.265 video codec. It provides
both an encoding and decoding API but the main objective of the library is to
provide h.265 decoding, as the encoding part is not actively developed [1]

This package was requested due to it being a dependency of libheif.
Note that libheif is developed by the same developer as libde265.

- CVE History
  - 48 CVEs since 2020.
  - The developer started to commit seriously to patching CVEs recently
    * Several security-only releases in the past months.
    * One of them patched 30 CVEs.
    * Reference to specific commits patching the CVE.
  - Issues were not addressed on time but recent upstream activity shows
    that the developer is now actively answering and fixing them.
- Build-Depends
  - All libraries are related to image processing, nothing out of normal.
- pre/post inst/rm scripts
  - None
- init scripts
  - None
- systemd units
  - None
- dbus services
  - None
- setuid binaries
  - None
- binaries in PATH
  - from the ".*examples" binary:
    - libde265-sherlock265: QT video player.
    - libde265-dec265: simple player for h.265 bitstreams.
- sudo fragments
  - None
- polkit files
  - None
- udev rules
  - None
- unit tests / autopkgtests
  - No unit tests available in the software.
  - No autopkgtests.
  - CI/CD in GitHub uses example input files to test (x86 and arm) as well as Valgrind + Coverity analysis.
- cron jobs
  - None
- Build logs
  - Some macro warnings.
  - Some libtool reminder warnings.
  - Some warnings related to exceeding maximum object size, depending on the input file.
- Processes spawned
  - No process spawned in the library, only on the tools and the examples.
- Memory management
  - The code performs checks to ensure malloc doesn't fail.
  - The code doesn't check the boundaries when calling memcpy (on internal functions only).
    Hard to trace if everything is checked before but many of the CVEs are because of assuming the right inputs.
  - Sprintfs are handled safely by using limits on the size.
- File IO
  - Library reads H.265/HEVC files to decode/encode them.
  - The API functions don't sanitize paths but it's a library so it should be checked by the frontend application.
  - No umask usage
- Logging
  - Done safely through local specific functions.
  - No possibility of format string.
- Environment variable usage
  - None
- Use of privileged functions
  - None
- Use of cryptography / random number sources etc
  - None
- Use of temp files
  - Only on the tools, not on the library.
- Use of networking
  - None
- Use of WebKit
  - None
- Use of PolicyKit
  - None

- Any significant cppcheck results
  - Mainly non-serious issues related to the encoder.
  - One issue related to memory management, no check in case of an error that will result in BOF.
- Any significant Coverity results
  - Nothing significant.
- Any significant shellcheck results
  - None
- Any significant bandit results
  - No Python code.
- Any significant govulncheck results
  - No Go code.
- Any significant Semgrep results
  - Nothing significant

The code seems to be a work in progress, with 195 TODO comments in master as of today, and lots of commented pieces of code. There is no security policy in place [2].

The developer uploads this software as deb packages in LP [3]. There is an extra package
called libde265-teststreams that contains examples and fuzzing cases that are also run during CI/CD. They could be used as autopkgtests. More examples can also be found in [4]

Releases are erratic, in the past 6 months the developer has published several security updates and other fixes but there was a 2-year gap from 2022 to 2020.

During the analysis, 2 vulnerabilities were discovered and reported [5][6]. The developer
quickly acknowledge and push fixes to the repository.

The encoding feature is not actively maintained and therefore this package
shouldn't be used for that purpose.

Security team ACK for promoting libde265 to main.

[1] https://github.com/strukturag/libheif/issues/797#event-8800456603
[2] https://github.com/strukturag/libde265/issues/420
[3] https://launchpad.net/~strukturag/+archive/ubuntu/libde265
[4] https://github.com/strukturag/libde265-data
[5] https://github.com/strukturag/libde265/issues/418
[6] https://github.com/strukturag/libde265/issues/419

Changed in libde265 (Ubuntu):
assignee:	Ubuntu Security Team (ubuntu-security) → nobody

Revision history for this message

Vladimir Petko (vpa1977) wrote on 2023-09-12:

#6

Work remaining:
- build and autopkgtests need to be implemented (planned this/next week)
- correct linitan warnings (optional)

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2023-09-19:

#7

Setting back to incomplete until it is ready for re-review for an ack (otherwise it shows up on the weekly checks)

Changed in libde265 (Ubuntu):
status:	New → Incomplete

Revision history for this message

Vladimir Petko (vpa1977) wrote on 2024-04-08:

#8

The smoke test for the decoder was implemented in 1.0.12-2, we do not use encoding functionality of libde265 for the MIR purposes.

This resolves the mandatory requirement for this package @slyon.

Revision history for this message

Lukas Märdian (slyon) wrote on 2024-04-09:

#9

FTR: autopkgtests were added via: https://bugs.debian.org/1052214

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2024-04-11:

#10

While only being a basic test and you never know if someone else comes later and says "ah in main, let us use for encoding" as well.
Just as guidance, those tests are never meant to be "you can only add the minimum" :-)
IMHO given what the lib does it still is fine for now and the infrastructure to extend it is present.

What we now have looks good in https://autopkgtest.ubuntu.com/packages/libd/libde265 for now.

This is ok to go AFAIC, but it only makes sense once the rest of the heif dependency tree is ready.
Setting the state while we wait for the rest to complete and then a change to pull it in.

Changed in libde265 (Ubuntu):
status:	Incomplete → In Progress

Lukas Märdian (slyon) on 2024-04-18

Changed in libde265 (Ubuntu):
status:	In Progress → Fix Committed

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2024-04-19:

#11

Ready

Full stack check in https://bugs.launchpad.net/ubuntu/+source/libheif/+bug/1827442/comments/55

The required exclude is in since https://git.launchpad.net/~ubuntu-core-dev/ubuntu-seeds/+git/ubuntu/commit/?id=640c0d5814139575e9839d533f5408e7ac43272a

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2024-04-19:

#12

In noble we only have one version atm:
libde265 | 1.0.15-1build3 | noble/universe | source

To promote src:libde265 bin:libde265-0

Override component to main
libde265 1.0.15-1build3 in noble: universe/misc -> main
Override [y|N]? y
1 publication overridden.

Override component to main
libde265-0 1.0.15-1build3 in noble amd64: universe/libs/optional/100% -> main
libde265-0 1.0.15-1build3 in noble arm64: universe/libs/optional/100% -> main
libde265-0 1.0.15-1build3 in noble armhf: universe/libs/optional/100% -> main
libde265-0 1.0.15-1build3 in noble i386: universe/libs/optional/100% -> main
libde265-0 1.0.15-1build3 in noble ppc64el: universe/libs/optional/100% -> main
libde265-0 1.0.15-1build3 in noble riscv64: universe/libs/optional/100% -> main
libde265-0 1.0.15-1build3 in noble s390x: universe/libs/optional/100% -> main
Override [y|N]? y
7 publications overridden.

Changed in libde265 (Ubuntu):
status:	Fix Committed → Fix Released

Ubuntu
libde265 package

[MIR] libde265 (dependency of libheif)

Bug Description

Other bug subscribers

Bug attachments

Remote bug watches

Ubuntulibde265 package

[MIR] libde265 (dependency of libheif)

Bug Description

Other bug subscribers

Bug attachments

Remote bug watches

Ubuntu
libde265 package