Fixing SAST reports

Resolves: RHEL-45003
This commit is contained in:
Lukas Javorsky 2024-08-21 09:28:19 +00:00
parent 2e929c5609
commit 110a111914
8 changed files with 229 additions and 1 deletions

View File

@ -0,0 +1,34 @@
From 73cf426409d3c9d097d650c7713c9d49d270623c Mon Sep 17 00:00:00 2001
From: Lukas Javorsky <ljavorsk@redhat.com>
Date: Thu, 1 Aug 2024 10:47:02 +0200
Subject: [PATCH] Fix the possible overrun of buf array
---
src/roff/troff/env.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/roff/troff/env.cpp b/src/roff/troff/env.cpp
index 1b3b4ba..8b91380 100644
--- a/src/roff/troff/env.cpp
+++ b/src/roff/troff/env.cpp
@@ -4003,7 +4003,7 @@ void hyphenate(hyphen_list *h, unsigned flags)
while (h && h->hyphenation_code == 0)
h = h->next;
int len = 0;
- char hbuf[WORD_MAX + 2];
+ char hbuf[WORD_MAX + 2 + 1];
char *buf = hbuf + 1;
hyphen_list *tem;
for (tem = h; tem && len < WORD_MAX; tem = tem->next) {
@@ -4063,7 +4063,7 @@ void hyphenate(hyphen_list *h, unsigned flags)
}
else {
hbuf[0] = hbuf[len + 1] = '.';
- int num[WORD_MAX + 3];
+ int num[WORD_MAX + 3 + 1];
current_language->patterns.hyphenate(hbuf, len + 2, num);
// The position of a hyphenation point gets marked with an odd
// number. Example:
--
2.45.2

View File

@ -0,0 +1,25 @@
From 2e548049590a455ce3824fa7950f03465b737501 Mon Sep 17 00:00:00 2001
From: Lukas Javorsky <ljavorsk@redhat.com>
Date: Thu, 1 Aug 2024 10:48:45 +0200
Subject: [PATCH 2/7] Fix for insufficient allocation of iterator
---
src/roff/troff/input.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/roff/troff/input.cpp b/src/roff/troff/input.cpp
index abf6198..18157a9 100644
--- a/src/roff/troff/input.cpp
+++ b/src/roff/troff/input.cpp
@@ -3630,9 +3630,10 @@ inline
#endif
temp_iterator::temp_iterator(const char *s, int len)
{
- base = new unsigned char[len];
+ base = new unsigned char[len + 1];
if (len > 0)
memcpy(base, s, len);
+ base[len] = '\0';
ptr = base;
eptr = base + len;
}

View File

@ -0,0 +1,28 @@
From a96155657b690a7de3a7670f3014867487cba838 Mon Sep 17 00:00:00 2001
From: Lukas Javorsky <ljavorsk@redhat.com>
Date: Mon, 12 Aug 2024 13:02:35 +0200
Subject: [PATCH 3/7] Safely handle the "name" argument in NewFile func
Since the NewFile func is called with argv[] arguments, there should be
a safety check in case that the arguments are tainted.
---
src/devices/xditview/xditview.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/devices/xditview/xditview.c b/src/devices/xditview/xditview.c
index 1f56940b2..9ee0470c2 100644
--- a/src/devices/xditview/xditview.c
+++ b/src/devices/xditview/xditview.c
@@ -364,7 +364,8 @@ NewFile (const char *name)
}
hadFile = 1;
SelectPageNumber ("1");
- strcpy (current_file_name, name);
+ strncpy(current_file_name, name, sizeof(current_file_name) - 1);
+ current_file_name[sizeof(current_file_name) - 1] = '\0'; // Ensure null-termination
current_file = new_file;
}
--
2.44.0

View File

@ -0,0 +1,26 @@
From d9e08e8d8687c32bc6f079fed91e708374aadde8 Mon Sep 17 00:00:00 2001
From: Lukas Javorsky <ljavorsk@redhat.com>
Date: Mon, 12 Aug 2024 14:38:00 +0200
Subject: [PATCH 4/7] Fix array comparison warning by comparing elements
individually
---
src/preproc/refer/ref.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/preproc/refer/ref.cpp b/src/preproc/refer/ref.cpp
index 5f6d5c63a..65b7edaf3 100644
--- a/src/preproc/refer/ref.cpp
+++ b/src/preproc/refer/ref.cpp
@@ -536,7 +536,7 @@ int same_reference(const reference &r1, const reference &r2)
return 0;
int i = 0;
for (i = 0; i < 256; i++)
- if (r1.field_index != r2.field_index)
+ if (r1.field_index[i] != r2.field_index[i])
return 0;
for (i = 0; i < r1.nfields; i++)
if (r1.field[i] != r2.field[i])
--
2.44.0

View File

@ -0,0 +1,36 @@
From c19274b6ec048aa8c9d5e78bec22609aadf0ff4c Mon Sep 17 00:00:00 2001
From: Lukas Javorsky <ljavorsk@redhat.com>
Date: Mon, 12 Aug 2024 15:45:45 +0200
Subject: [PATCH 5/7] Initialize "s" to prevent undefined behavior
---
src/roff/troff/env.cpp | 2 +-
src/roff/troff/input.cpp | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/roff/troff/env.cpp b/src/roff/troff/env.cpp
index 04eedd1..7d1c8d1 100644
--- a/src/roff/troff/env.cpp
+++ b/src/roff/troff/env.cpp
@@ -287,7 +287,7 @@ void leader_character()
void environment::add_char(charinfo *ci)
{
- int s;
+ int s = 0;
node *gc_np = 0;
if (interrupted)
;
diff --git a/src/roff/troff/input.cpp b/src/roff/troff/input.cpp
index bd586cb..4308926 100644
--- a/src/roff/troff/input.cpp
+++ b/src/roff/troff/input.cpp
@@ -7300,7 +7300,7 @@ void check_missing_character()
int token::add_to_zero_width_node_list(node **pp)
{
hunits w;
- int s;
+ int s = 0;
node *n = 0;
switch (type) {
case TOKEN_CHAR:

View File

@ -0,0 +1,36 @@
From afd4d3247df46f5ec21a86627203234f37d750ee Mon Sep 17 00:00:00 2001
From: Lukas Javorsky <ljavorsk@redhat.com>
Date: Mon, 12 Aug 2024 15:52:14 +0200
Subject: [PATCH 6/7] Initialize "x" and "y" elements of the "here" structure
Using uninitialized variables in the "path::follow" function could cause
an undefined behavior.
---
src/preproc/pic/object.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/preproc/pic/object.cpp b/src/preproc/pic/object.cpp
index d0b648c27..b197a02fa 100644
--- a/src/preproc/pic/object.cpp
+++ b/src/preproc/pic/object.cpp
@@ -891,6 +891,8 @@ int object_spec::position_rectangle(rectangle_object *p,
if (flags & HAS_WITH) {
place offset;
place here;
+ here.x = 0;
+ here.y = 0;
here.obj = p;
if (!with->follow(here, &offset))
return 0;
@@ -1512,6 +1514,8 @@ linear_object *object_spec::make_line(position *curpos, direction *dirp)
position pos = at;
place offset;
place here;
+ here.x = 0;
+ here.y = 0;
here.obj = &tmpobj;
if (!with->follow(here, &offset))
return 0;
--
2.44.0

View File

@ -0,0 +1,25 @@
From 02e7914f70f3afb37b5ebadc65da35e5df47ea8e Mon Sep 17 00:00:00 2001
From: Lukas Javorsky <ljavorsk@redhat.com>
Date: Mon, 12 Aug 2024 16:14:40 +0200
Subject: [PATCH 7/7] Fix uninitialized memory usage in override_sizes by
zero-initializing sizes array
If `strtok` returns `null`, we break early from for-loop before initializing any values to sizes. We then access uninitialized values. Only other case where we break out of the loop is when `lower` is 0, and we do only after adding this 0 to `sizes`. Function `init_size_table` uses "\0" to detect end of the array, so in this case we shouldn't be accessing any uninitialized values.
---
src/roff/troff/env.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/roff/troff/env.cpp b/src/roff/troff/env.cpp
index 62c251927..b54df35e9 100644
--- a/src/roff/troff/env.cpp
+++ b/src/roff/troff/env.cpp
@@ -1318,7 +1318,8 @@ void point_size()
void override_sizes()
{
int n = 16;
- int *sizes = new int[n];
+ int *sizes = new int[n]; // C++03: new int[n]();
+ (void) memset(sizes, 0, (n * sizeof(int)));
int i = 0;
char *buf = read_string();
if (!buf)

View File

@ -3,7 +3,7 @@
Summary: A document formatting system
Name: groff
Version: 1.23.0
Release: 7%{?dist}
Release: 8%{?dist}
# Everything is under GPL-3.0-or-later, except for the following files:
# MIT license
# -- tmac/hyphen.den
@ -59,7 +59,22 @@ Patch3: 0004-don-t-use-usr-bin-env-in-shebang.patch
Patch4: 0005-do-not-overwrite-docdir.patch
# Revert upstream change of mapping special characters for UTF-8 devices
# Debian commit: https://salsa.debian.org/debian/groff/-/commit/d5394c68d70e6c5199b01d2522e094c8fd52e64e
# SAST fixes
# Reported to upstream:
# https://savannah.gnu.org/bugs/?66052
# https://savannah.gnu.org/bugs/index.php?66076
# https://savannah.gnu.org/bugs/index.php?66078
# https://savannah.gnu.org/bugs/index.php?66079
# https://savannah.gnu.org/bugs/index.php?66080
# https://savannah.gnu.org/bugs/?66081
Patch5: 0006-Revert-upstream-change-of-mapping-special-characters.patch
Patch6: 0007-Fix-the-possible-overrun-of-buf-array.patch
Patch7: 0008-Fix-for-insufficient-allocation-of-iterator.patch
Patch8: 0009-Safely-handle-the-name-argument-in-NewFile-func.patch
Patch9: 0010-Fix-array-comparison-warning-by-comparing-elements-i.patch
Patch10: 0011-Initialize-s-to-prevent-undefined-behavior.patch
Patch11: 0012-Initialize-x-and-y-elements-of-the-here-structure.patch
Patch12: 0013-Fix-uninitialized-memory-usage-in-override_sizes-by-.patch
Requires: coreutils, groff-base = %{version}-%{release}
@ -500,6 +515,9 @@ fi
%doc %{_pkgdocdir}/pdf/
%changelog
* Wed Aug 21 2024 Lukas Javorsky <ljavorsk@redhat.com> - 1.23.0-8
- Fix SAST identified true positives
* Mon Jun 24 2024 Troy Dawson <tdawson@redhat.com> - 1.23.0-7
- Bump release for June 2024 mass rebuild