[Debian-med-packaging] Bug#1075990: bamtools corrupts output data on bigendian architectures

Vladimir Petko vpa1977 at gmail.com
Fri Jul 12 04:11:45 BST 2024


Package: bamtools
Version: 2.5.2+dfsg-5
Followup-For: Bug #1075990
User: ubuntu-devel at lists.ubuntu.com
Usertags: origin-ubuntu oracular ubuntu-patch
Control: tags -1 patch

Dear Maintainer,

Would it be possible to consider adding attached patches to resolve infinite
loop in the filter script loader (non-amd) and the output corruption of S390?

In Ubuntu, the attached patch was applied to achieve the following:
  * d/p/do_not_corrupt_output.patch: add patch to avoid corrupting
    output on big-endian platforms (LP: #2072463).
  * d/p/filter_script.patch: add patch to address Debian bug 992143.
  * d/t/run-unit-test: enable filter test on all architectures.


Thanks for considering the patch.


-- System Information:
Debian Release: trixie/sid
  APT prefers noble-updates
  APT policy: (500, 'noble-updates'), (500, 'noble-security'), (500, 'noble'), (100, 'noble-backports')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 6.8.0-36-generic (SMP w/32 CPU threads; PREEMPT)
Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled
-------------- next part --------------
diff -Nru bamtools-2.5.2+dfsg/debian/patches/do_not_corrupt_output.patch bamtools-2.5.2+dfsg/debian/patches/do_not_corrupt_output.patch
--- bamtools-2.5.2+dfsg/debian/patches/do_not_corrupt_output.patch	1970-01-01 12:00:00.000000000 +1200
+++ bamtools-2.5.2+dfsg/debian/patches/do_not_corrupt_output.patch	2024-07-09 15:57:31.000000000 +1200
@@ -0,0 +1,36 @@
+Description: bamtools crashes/corrupts output data on s390x
+ The issue was detected in Ubuntu autopkgtests. The call to
+ bamtools revert -in sam_spec_example.bam -out out.bam
+ fails due to the buffer overflow detected
+ This is due to the write loop in
+ src/api/internal/bam/BamWriter_p.cpp
+ using single byte instead of sizeof(uint32_t) increment to
+ swap bytes in the integer data.
+ The output file on s390x is corrupted by the write operation.
+ bamtools crash with the hardening flags enabled.
+Author: Vladimir Petko <vladimir.petko at canonical.com>
+Bug: https://github.com/pezmaster31/bamtools/issues/235
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/bamtools/+bug/2072463
+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1075990
+Last-Update: 2024-07-09
+
+--- a/src/api/internal/bam/BamWriter_p.cpp
++++ b/src/api/internal/bam/BamWriter_p.cpp
+@@ -349,7 +349,7 @@
+             char* cigarData = new char[packedCigarLength]();
+             std::memcpy(cigarData, packedCigar.data(), packedCigarLength);
+             if (m_isBigEndian) {
+-                for (size_t i = 0; i < packedCigarLength; ++i) {
++                for (size_t i = 0; i < packedCigarLength; i+= sizeof(uint32_t)) {
+                     BamTools::SwapEndian_32p(&cigarData[i]);
+                 }
+             }
+@@ -501,7 +501,7 @@
+             std::memcpy(cigarData, packedCigar.data(), packedCigarLength);
+             if (m_isBigEndian) {
+                 for (size_t i = 0; i < packedCigarLength;
+-                     ++i) {  // FIXME: similarly, this should be "i += 4", not "++i"
++                     i+= sizeof(uint32_t)) {  // FIXME: similarly, this should be "i += 4", not "++i"
+                     BamTools::SwapEndian_32p(&cigarData[i]);
+                 }
+             }
diff -Nru bamtools-2.5.2+dfsg/debian/patches/filter_script.patch bamtools-2.5.2+dfsg/debian/patches/filter_script.patch
--- bamtools-2.5.2+dfsg/debian/patches/filter_script.patch	1970-01-01 12:00:00.000000000 +1200
+++ bamtools-2.5.2+dfsg/debian/patches/filter_script.patch	2024-07-09 15:57:31.000000000 +1200
@@ -0,0 +1,43 @@
+Description: fix infinite loop on s390x, arm, ppc64el
+ const std::string FilterTool::FilterToolPrivate::GetScriptContents()
+ loops indefinitely due to fgets() not setting eof flag if
+ the call returns data on those platforms.
+ The fgetc/ungetc calls then set/clear eof flag indefinitely.
+Author: Vladimir Petko <vladimir.petko at canonical.com>
+Bug: https://github.com/pezmaster31/bamtools/issues/237
+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=992143
+Last-Update: 2024-07-09
+
+diff --git a/src/toolkit/bamtools_filter.cpp b/src/toolkit/bamtools_filter.cpp
+index 16a1b0d..9c2ea44 100644
+--- a/src/toolkit/bamtools_filter.cpp
++++ b/src/toolkit/bamtools_filter.cpp
+@@ -540,22 +540,18 @@ const std::string FilterTool::FilterToolPrivate::GetScriptContents()
+     // read in entire script contents
+     char buffer[1024];
+     std::ostringstream docStream;
+-    while (true) {
+-
+-        // peek ahead, make sure there is data available
+-        char ch = fgetc(inFile);
+-        ungetc(ch, inFile);
+-        if (feof(inFile)) {
++    while (!feof(inFile)) {
++        // read next block of data
++        char *data = fgets(buffer, 1024, inFile);
++        if (data == 0) {
+             break;
+         }
+-
+-        // read next block of data
+-        if (fgets(buffer, 1024, inFile) == 0) {
++        if (ferror(inFile)) {
+             std::cerr << "bamtools filter ERROR: could not read script contents" << std::endl;
+             return std::string();
+         }
+ 
+-        docStream << buffer;
++        docStream << data;
+     }
+ 
+     // close script file
diff -Nru bamtools-2.5.2+dfsg/debian/patches/series bamtools-2.5.2+dfsg/debian/patches/series
--- bamtools-2.5.2+dfsg/debian/patches/series	2023-12-16 07:09:45.000000000 +1300
+++ bamtools-2.5.2+dfsg/debian/patches/series	2024-07-09 15:57:31.000000000 +1200
@@ -2,3 +2,5 @@
 shared_and_static.patch
 #fix_soversion.patch
 typo.patch
+filter_script.patch
+do_not_corrupt_output.patch
diff -Nru bamtools-2.5.2+dfsg/debian/tests/run-unit-test bamtools-2.5.2+dfsg/debian/tests/run-unit-test
--- bamtools-2.5.2+dfsg/debian/tests/run-unit-test	2023-12-16 07:09:45.000000000 +1300
+++ bamtools-2.5.2+dfsg/debian/tests/run-unit-test	2024-07-09 15:57:31.000000000 +1200
@@ -21,19 +21,7 @@
 
 bamtools coverage -in sam_spec_example.bam -out out
 
-# This test fails on ppc64el for whatever reason and is for the moment (see bug #933505)
-# The test is also problematic for armel (see bug #992143)
-ARCH=$(dpkg --print-architecture)
-if [ "$ARCH" != "ppc64el" -a "$ARCH" != "arm64" -a "$ARCH" != "armel"-a "$ARCH" != "armhf" -a "$ARCH" != "s390x"  ] ; then
-   bamtools filter -script filter_script -in sam_spec_example.bam -out out.bam
-else
-   if [ "$ARCH" = "ppc64el" ] ; then
-      echo "The following test is known to fail on ppc64el architecture (see bug #933505)"
-   else
-      echo "The following test is known to time out on $ARCH architecture (see bug #953939)"
-   fi
-   echo "bamtools filter -script filter_script -in sam_spec_example.bam -out out.bam"
-fi
+bamtools filter -script filter_script -in sam_spec_example.bam -out out.bam
 
 bamtools header -in sam_spec_example.bam
 


More information about the Debian-med-packaging mailing list