Merge ~pushkarnk/ubuntu/+source/aevol:fix-buffer-overflow into ubuntu/+source/aevol:ubuntu/devel

Proposed by Pushkar Kulkarni
Status: Needs review
Proposed branch: ~pushkarnk/ubuntu/+source/aevol:fix-buffer-overflow
Merge into: ubuntu/+source/aevol:ubuntu/devel
Diff against target: 77 lines (+46/-1)
4 files modified
debian/changelog (+7/-0)
debian/control (+2/-1)
debian/patches/fix-buffer-overflow.patch (+36/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
Vladimir Petko (community) Needs Fixing
git-ubuntu import Pending
Review via email: mp+468397@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Pushkar Kulkarni (pushkarnk) wrote :

Test builds in PPA:

Local autopkgtest run

=====

autopkgtest
  --shell-fail \
  --setup-commands="sudo apt update && \
      sudo apt install software-properties-common -y && \
      sudo add-apt-repository -y -u -s ppa:pushkarnk/test-builds" \
  aevol \
   -- lxd autopkgtest/ubuntu/oracular/amd64

...
...
...

WARNING : Parameter change during simulation is not managed in general.
          Only changes in environmental target done with aevol_modify are handled.

autopkgtest [11:04:52]: test run-unit-tests: -----------------------]
autopkgtest [11:04:52]: test run-unit-tests: - - - - - - - - - - results - - - - - - - - - -
run-unit-tests PASS
autopkgtest [11:04:53]: @@@@@@@@@@@@@@@@@@@@ summary
run-unit-tests PASS

=====

Revision history for this message
Vladimir Petko (vpa1977) :
review: Approve
Revision history for this message
Vladimir Petko (vpa1977) wrote (last edit ):

When strchr returns null, sprintf tries to print `0(null)` so we need a sufficient buffer for this situation. If we check that string starts with space and skip those we kind of change the output, but the output is not really informative.
If the line starts with space, then we would be appending extra character to the string and we need +2 buffer for it instead of +1.

Conclusion: we might want to check strchr() output and use +2 buffer to avoid overflow.

review: Needs Fixing

Unmerged commits

8eaaf57... by Pushkar Kulkarni

update maintainer

1f8d62c... by Pushkar Kulkarni

update changelog

8c019b3... by Pushkar Kulkarni

Add patch to fix a buffer overflow

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index f982c5b..1b4e5d5 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,10 @@
6+aevol (5.0+ds-3ubuntu1) oracular; urgency=medium
7+
8+ * d/patches: Add patch to fix a buffer overflow
9+ (LP: #2071412)
10+
11+ -- Pushkar Kulkarni <pushkar.kulkarni@canonical.com> Fri, 28 Jun 2024 10:27:54 +0530
12+
13 aevol (5.0+ds-3build1) noble; urgency=medium
14
15 * No-change rebuild for boost defaults change.
16diff --git a/debian/control b/debian/control
17index 42bcc60..bca0d15 100644
18--- a/debian/control
19+++ b/debian/control
20@@ -1,5 +1,6 @@
21 Source: aevol
22-Maintainer: Debian Med Packaging Team <debian-med-packaging@lists.alioth.debian.org>
23+Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
24+XSBC-Original-Maintainer: Debian Med Packaging Team <debian-med-packaging@lists.alioth.debian.org>
25 Uploaders: David Parsons <david.parsons@inria.fr>
26 Section: science
27 Priority: optional
28diff --git a/debian/patches/fix-buffer-overflow.patch b/debian/patches/fix-buffer-overflow.patch
29new file mode 100644
30index 0000000..d34b0d1
31--- /dev/null
32+++ b/debian/patches/fix-buffer-overflow.patch
33@@ -0,0 +1,36 @@
34+Description: Patch to fix buffer overflows caused by a NULL
35+ passed to sprintf() in its argument list. It looks like
36+ the code expects the propagated_timestep line to begin with
37+ a non-whitespace character (most likely a number). If the
38+ line begins with a whitespace or is empty, a buffer overflow
39+ results when compiled with -O2. This patch is not relevant
40+ to the current upstream which is at v9, while the Debian/Ubuntu
41+ package still points to v5.
42+Forwarded: non-needed
43+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/aevol/+bug/2071412
44+Author: Pushkar Kulkarni <pushkar.kulkarni@canonical.com>
45+--- a/src/libaevol/Stats.cpp
46++++ b/src/libaevol/Stats.cpp
47+@@ -32,6 +32,7 @@
48+ // =================================================================
49+ #include "Stats.h"
50+
51++#include <cctype>
52+ #include <string>
53+
54+ #include <err.h>
55+@@ -807,9 +808,11 @@
56+
57+ // Write the line for propagated_timestep
58+ // after replacing the timestep with 0
59+- char new_line[strlen(line)+1];
60+- sprintf(new_line, "0%s", strchr(line, ' '));
61+- fputs(new_line, new_file);
62++ if (strlen(line) > 0 && !isspace(line[0])) { //avoid buffer overflow crashes
63++ char new_line[strlen(line)+1];
64++ sprintf(new_line, "0%s", strchr(line, ' '));
65++ fputs(new_line, new_file);
66++ }
67+
68+ fclose(old_file);
69+ fclose(new_file);
70diff --git a/debian/patches/series b/debian/patches/series
71index 64024c9..d2be0ae 100644
72--- a/debian/patches/series
73+++ b/debian/patches/series
74@@ -1,2 +1,3 @@
75 automake.patch
76 groff-message.patch
77+fix-buffer-overflow.patch

Subscribers

People subscribed via source and target branches