distribution-gpg-keys-copr crashes Launchpad/apt-ftparchive

Bug #2083697 reported by Julian Andres Klode
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
apt (Ubuntu)
Fix Released
Undecided
Unassigned
Focal
Fix Released
Undecided
Unassigned
Jammy
Fix Released
Undecided
Unassigned
Noble
Fix Released
Undecided
Unassigned
Oracular
Fix Released
Undecided
Unassigned
distribution-gpg-keys (Ubuntu)
Fix Released
Undecided
Unassigned
Oracular
Fix Released
Undecided
Unassigned

Bug Description

[Impact]
apt-ftparchive used a custom tree data structure and statically sized buffers, causing

1. buffer overflows in the statically sized buffers
2. exponential complexity on insertion as the per-directory binary trees were unbalanced (and debs are sorted, so they _always_ cause exponential complexity, building a linked list), causing contents generation to take hours instead of seconds.
3. stack overflow by recursion when trying to generate Contents for oracular with distribution-gpg-keys-copr included (as we are recursing the tree on the stack, we were over 30k stack frames deep at a cursory check of distribution-gpg-keys-copr alone).

This can lead to crashes and hence denial of service in apt-ftparchive when generating Contents files. The denial of service is not of significant concern, as it only affects a single repository and owners of repositories must have reasonable trust in the packages in said repositories, otherwise they would not be accepting them and plan to offer them to clients. An easier and more worthwhile denial of service can be achieved using zip bombs, that is, compressing multiple TBs of zeroes inside the deb, leading apt-ftparchive to spend hours in the Contents generation decompressing and ignoring the file data at probably 100% CPU usage.

This does not affect the apt library, nor does it affect other bits of apt-ftparchive outside the contents generation.

Hence we see the value of this mainly in functional terms, both making it significant faster and able to work with many files in the same directory, or deep file paths, in the first place.

[Test Plan]
The autopkgtests should prevent any regressions. We have added additional checks for apt-ftparchive contents, checking deep directories and directories with many files with valgrind. These also in particular check the correctness of the output of the Contents file generation.

The directory with many files did not cause a crash previously locally, it's unclear how to exactly reproduce the launchpad side; it probably needs the exact same set of debs as the Ubuntu archive.

[Where problems could occur]
We have rewritten the Contents file generation, removing the broken custom search tree in favor of a simple std::set of (path, package) pairs (where paths and packages are allocated in larger blocks for memory efficiency).

One notable change in behavior is that the list of packages is now sorted. It should be considered a bug that the list of packages was not ordered before, but it is a change in behavior.

[Other information]
Be advised that this is hard to review as a diff, given that it removes the old
implementation and adds the new one but keeps the function names. Particularly GenContents::Print() diff is sadly broken up into multiple chunks. It may be more suitable to just look at the new GenContents::Print() instead.

We have increased the size of the memory pools from 40960 byte to 4 MiB and added an abort() if we were to run out of memory there, so there still is a limit for path and package names, we do not anticipate reaching that though.

A simple change to apt-pkg/pkgcachegen.cc is included to pacify valgrind as needed for the stronger valgrind testing integration that is used to verify no buffer overflows in the test-apt-ftparchive-corner-cases test, as otherwise the other test using valgrind would fail.

Revision history for this message
Julian Andres Klode (juliank) wrote :

This bug should remain open until apt is fixed, it's not clear what the security impact is.

Revision history for this message
Julian Andres Klode (juliank) wrote :

We believed this was an overflow in the static filename buffer but looking again I believe this is an issue with the number of the files in a directory, as we traverse all files in the archive, recursively, they are in a natural directory layout.

Changed in distribution-gpg-keys (Ubuntu):
status: New → Fix Released
description: updated
no longer affects: distribution-gpg-keys (Ubuntu Focal)
no longer affects: distribution-gpg-keys (Ubuntu Jammy)
no longer affects: distribution-gpg-keys (Ubuntu Noble)
description: updated
description: updated
Revision history for this message
Julian Andres Klode (juliank) wrote :

The cat is out of the bag. This is fixed in 2.9.9 and will be handled via regular stable release updates.

information type: Private Security → Public Security
Revision history for this message
Julian Andres Klode (juliank) wrote :

Unsubscribed the SRU people I added to give them visibility while it was private, and updated the description with more impact details.

description: updated
description: updated
description: updated
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apt - 2.9.14ubuntu1

---------------
apt (2.9.14ubuntu1) plucky; urgency=medium

  [ David Kalnischkies ]
  * Collect unprinted Ign errors for display in Err output

  [ Julian Andres Klode ]
  * test-apt-cdrom: Hande assert-pubkey-algo like other gpgv messages
  * Do not use apt-key anymore
    - gpgv: Generalize apt_error to apt_msg(), add apt_warning()
    - Do not implode key file name vector for calling gpgv
    - apt-key: Only cat supported keyrings into the merged one
    - apt-key: Temporarily accept 'pub' as an extension for binary gpg keys
    - gpgv: Explictly pass all the keyrings to gpgv from apt
    - gpgv: Use std::string instead of const char *
    - strutl: Add Base64Decode
    - gpgv: Verify keyrings and dearmor outside apt-key
    - Directly call gpgv instead of apt-key
    - gpgv: Add direct support for --assert-pubkey-algo
    - Remove the apt-key manual page and add documentation to apt-secure
   (same as 2.9.14+noAptKey1, but "Stop installing apt-key, make it a test
    suite helper" reverted)

 -- Julian Andres Klode <email address hidden> Sun, 24 Nov 2024 15:11:02 +0100

Changed in apt (Ubuntu):
status: New → Fix Released
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

I'm not super familiar with apt-ftparchive. Going through the description in the bug, and manpage, I see mention on a cache db.

Do the changes here in this bug invalidate existing cache db files? I see that the list of packages is now ordered, does that affect the caching?

In other words, some repo which has this cache db from a previous version of apt-ftparchive, and now gets the SRU version, is that cache invalidated? Rebuilt from scratch? Or worse, broken?

Revision history for this message
Andreas Hasenack (ahasenack) wrote :
Revision history for this message
Julian Andres Klode (juliank) wrote :

The runtime tree structure/contents mapping is fed from a cache db which just essentially maps package,type pairs to some data (e.g. (package, contents ) maps to the list of files) or the packages directly. At runtime we read the file list from each file and build the reverse map file->packages.

The cache db is not affected by this or other changes in later versions of apt for that matter.

A future release may move the cache db from bdb to lmdb to improve reliability, but such a change would end up being transparent, except for a one-time performance hit when the old database is discarded :D

Revision history for this message
Andreas Hasenack (ahasenack) wrote : Please test proposed package

Hello Julian, or anyone else affected,

Accepted apt into oracular-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/apt/2.9.8ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-oracular to verification-done-oracular. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-oracular. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in apt (Ubuntu Oracular):
status: New → Fix Committed
tags: added: verification-needed verification-needed-oracular
Revision history for this message
Ubuntu SRU Bot (ubuntu-sru-bot) wrote : Autopkgtest regression report (apt/2.9.8ubuntu0.1)

All autopkgtests for the newly accepted apt (2.9.8ubuntu0.1) for oracular have finished running.
The following regressions have been reported in tests triggered by the package:

apt/unknown (ppc64el)
auto-apt-proxy/unknown (ppc64el)
cron/unknown (ppc64el)
dgit/11.10 (amd64)
dgit/unknown (ppc64el)
gcc-14/unknown (ppc64el)
gcc-snapshot/1:20241004-1ubuntu1 (ppc64el)
sbuild/0.85.10ubuntu1 (s390x)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/oracular/update_excuses.html#apt

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

Revision history for this message
Andreas Hasenack (ahasenack) wrote : Please test proposed package

Hello Julian, or anyone else affected,

Accepted apt into noble-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/apt/2.8.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-noble to verification-done-noble. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-noble. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in apt (Ubuntu Noble):
status: New → Fix Committed
tags: added: verification-needed-noble
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Hello Julian, or anyone else affected,

Accepted apt into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/apt/2.4.14 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in apt (Ubuntu Jammy):
status: New → Fix Committed
tags: added: verification-needed-jammy
Changed in apt (Ubuntu Focal):
status: New → Fix Committed
tags: added: verification-needed-focal
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Hello Julian, or anyone else affected,

Accepted apt into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/apt/2.0.11 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Revision history for this message
Ubuntu SRU Bot (ubuntu-sru-bot) wrote : Autopkgtest regression report (apt/2.0.11)

All autopkgtests for the newly accepted apt (2.0.11) for focal have finished running.
The following regressions have been reported in tests triggered by the package:

apport/2.20.11-0ubuntu27.27 (arm64)
autopkgtest/unknown (ppc64el)
dgit/9.10 (arm64)
gcc-10/10.5.0-1ubuntu1~20.04 (ppc64el)
unattended-upgrades/2.3ubuntu0.3 (ppc64el)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/focal/update_excuses.html#apt

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

Revision history for this message
Ubuntu SRU Bot (ubuntu-sru-bot) wrote : Autopkgtest regression report (apt/2.4.14)

All autopkgtests for the newly accepted apt (2.4.14) for jammy have finished running.
The following regressions have been reported in tests triggered by the package:

dgit/9.15 (amd64, ppc64el)
gcc-11/11.4.0-1ubuntu1~22.04 (ppc64el)
gcc-12/12.3.0-1ubuntu1~22.04 (ppc64el)
gcc-9/9.5.0-1ubuntu1~22.04 (ppc64el)
livecd-rootfs/2.765.54 (arm64)
sbuild/unknown (ppc64el)
ubuntu-advantage-tools/unknown (ppc64el)
unattended-upgrades/2.8ubuntu1 (ppc64el)
update-notifier/unknown (ppc64el)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/jammy/update_excuses.html#apt

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

Revision history for this message
Ubuntu SRU Bot (ubuntu-sru-bot) wrote : Autopkgtest regression report (apt/2.8.3)

All autopkgtests for the newly accepted apt (2.8.3) for noble have finished running.
The following regressions have been reported in tests triggered by the package:

auto-apt-proxy/14.1 (s390x)
autopkgtest/5.38ubuntu1~24.04.1 (armhf)
ca-certificates-java/20240118 (ppc64el)
cron/3.0pl1-184ubuntu2 (ppc64el)
dgit/11.8 (ppc64el, s390x)
gcc-10/10.5.0-4ubuntu2 (ppc64el)
gcc-11/unknown (ppc64el)
gcc-12/unknown (ppc64el)
gcc-13/13.3.0-6ubuntu2~24.04 (ppc64el)
gcc-14/14.2.0-4ubuntu2~24.04 (ppc64el)
gcc-9/9.5.0-6ubuntu2 (ppc64el)
gcc-snapshot/unknown (ppc64el)
update-manager/1:24.04.9 (amd64, arm64, armhf, i386, ppc64el, s390x)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/noble/update_excuses.html#apt

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

Revision history for this message
Julian Andres Klode (juliank) wrote :

The autopkgtests for APT have passed on all releases, so marking the bug as verified per the test plan (as the test suite includes the tests for this bug).

All regressions except for update-manager/noble have been resolved by retries. The update-manager regressions in noble are caused by the changes for bug 2060721 (see comments #27 and #28); a fix for update-manager's test suite is queued in unapproved (bug 2109339).

tags: added: verification-done verification-done-focal verification-done-jammy verification-done-noble verification-done-oracular
removed: verification-needed verification-needed-focal verification-needed-jammy verification-needed-noble verification-needed-oracular
Revision history for this message
Timo Aaltonen (tjaalton) wrote : Update Released

The verification of the Stable Release Update for apt has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apt - 2.9.8ubuntu0.1

---------------
apt (2.9.8ubuntu0.1) oracular; urgency=medium

  * Fix buffer overflow, stack overflow, exponential complexity in
    apt-ftparchive Contents generation (LP: #2083697)
    - ftparchive: Mystrdup: Add safety check and bump buffer size
    - ftparchive: contents: Avoid exponential complexity and overflows
    - test framework: Improve valgrind support
    - test: Check that apt-ftparchive handles deep paths
  * Workaround valgrind "invalid read" in ExtractTar::Go by moving large
    buffer from stack to heap. The large buffer triggered some bugs in
    valgrind stack clash protection handling.
  * debian/gbp.conf: Point at oracular branch

 -- Julian Andres Klode <email address hidden> Tue, 22 Oct 2024 14:54:15 +0200

Changed in apt (Ubuntu Oracular):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apt - 2.8.3

---------------
apt (2.8.3) noble; urgency=medium

  * Revert increased key size requirements from 2.8.0-2.8.2 (LP: #2073126)
    - Revert "Only install 00-temporary-rsa1024 for >=2.7.6 and improve comment"
    - Revert "Only warn about <rsa2048 when upgrading from 2.7.x to 2.8.x"
    - Revert rsa1024 to warnings again
    This leaves the mechanisms in place and no longer warns about NIST curves.
  * Fix keeping back removals of obsolete packages; and return an error if
    ResolveByKeep() is unsuccessful (LP: #2078720)
  * Fix buffer overflow, stack overflow, exponential complexity in
    apt-ftparchive Contents generation (LP: #2083697)
    - ftparchive: Mystrdup: Add safety check and bump buffer size
    - ftparchive: contents: Avoid exponential complexity and overflows
    - test framework: Improve valgrind support
    - test: Check that apt-ftparchive handles deep paths
    - Workaround valgrind "invalid read" in ExtractTar::Go by moving large
      buffer from stack to heap. The large buffer triggered some bugs in
      valgrind stack clash protection handling.

apt (2.8.2) noble; urgency=medium

  * Only install 00-temporary-rsa1024 for >=2.7.6 and improve comment
    (follow-up for LP: #2073126)

apt (2.8.1) noble; urgency=medium

  * Only revoke weak RSA keys for now, add 'next' and 'future' levels
    (backported from 2.9.7)
    Note that the changes to warn about keys not matching the future level
    in the --audit level are not fully included, as the --audit feature
    has not yet been backported. (LP: #2073126)
  * Introduce further mitigation on upgrades from 2.7.x to allow these
    systems to continue using rsa1024 repositories with warnings
    until the 24.04.2 point release (LP: #2073126)

apt (2.8.0) noble; urgency=medium

  [ Julian Andres Klode ]
  * Revert "Temporarily downgrade key assertions to "soon worthless""
    We temporarily downgraded the errors to warnings to give the
    launchpad PPAs time to be fixed, but warnings are not safe:
    Untrusted keys could be hiding on your system, but just not
    used at the moment. Hence revert this so we get the errors we
    want. (LP: #2060721)
  * Branch off the stable 2.8.y branch for noble:
    - CI: Test in ubuntu:noble images for 2.8.y
    - debian/gbp.conf: Point at the 2.8.y branch

  [ David Kalnischkies ]
  * Test suite fixes:
    - Avoid subshell hiding failure report from testfilestats
    - Ignore umask of leftover diff_Index in failed pdiff test
  * Documentation translation fixes:
    - Fix and unfuzzy previous VCG/Graphviz URI change

 -- Julian Andres Klode <email address hidden> Tue, 22 Oct 2024 15:02:22 +0200

Changed in apt (Ubuntu Noble):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apt - 2.4.14

---------------
apt (2.4.14) jammy; urgency=medium

  * Fix buffer overflow, stack overflow, exponential complexity in
    apt-ftparchive Contents generation (LP: #2083697)
    - ftparchive: Mystrdup: Add safety check and bump buffer size
    - ftparchive: contents: Avoid exponential complexity and overflows
    - test framework: Improve valgrind support
    - test: Check that apt-ftparchive handles deep paths
    - increase valgrind cleanliness to make the tests pass
      - pkgcachegen: Use placement new to construct header
      - Workaround valgrind "invalid read" in ExtractTar::Go by moving large
        buffer from stack to heap. The large buffer triggered some bugs in
        valgrind stack clash protection handling.

 -- Julian Andres Klode <email address hidden> Tue, 22 Oct 2024 15:09:58 +0200

Changed in apt (Ubuntu Jammy):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apt - 2.0.11

---------------
apt (2.0.11) focal; urgency=medium

  * Fix buffer overflow, stack overflow, exponential complexity in
    apt-ftparchive Contents generation (LP: #2083697)
    - ftparchive: Mystrdup: Add safety check and bump buffer size
    - ftparchive: contents: Avoid exponential complexity and overflows
    - test framework: Improve valgrind support
    - test: Check that apt-ftparchive handles deep paths
    - increase valgrind cleanliness to make the tests pass:
      - pkgcachegen: Use placement new to construct header
      - acquire: Disable gcc optimization of strcmp() reading too far into
        struct dirent's d_name buffer.

 -- Julian Andres Klode <email address hidden> Tue, 22 Oct 2024 15:27:19 +0200

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

Other bug subscribers

Remote bug watches

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