[PATCH] sparc_attrs.c: Prevent buffer overflow in sparc_check_object_attribute
first report of the static analyzer:
A string is copied into the buffer 's' of size 577 without checking its length
first at sparc_attrs.c:95.
Corrections explained:
Record Length Limit: We use strncat to add a line indicating the available
remaining_size. This prevents writing beyond the allocated memory.
Remaining space update: remaining_size is updated after each entry to ensure
that row additions do not cause overflow.
Found by RASU JSC.
Signed-off-by: Anton Moryakov
---
elfutils/backends/sparc_attrs.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/elfutils/backends/sparc_attrs.c b/elfutils/backends/sparc_attrs.c
index 974e8fb..0ba17c8 100644
--- a/elfutils/backends/sparc_attrs.c
+++ b/elfutils/backends/sparc_attrs.c
@@ -87,12 +87,17 @@ sparc_check_object_attribute (Ebl *ebl __attribute__
((unused)),
}
char *s = name;
+ size_t remaining_size = sizeof(name) - 1;
for (cap = 0; cap < 32; cap++)
if (value & (1U << cap))
{
-if (*s != '\0')
- s = strcat (s, ",");
-s = strcat (s, caps[cap]);
+if (*s != '\0'&& remaining_size > 1)
+{
+ strncat(s, ",", remaining_size);
+ remaining_size --;
+}
+strncat(s, caps[cap], remaining_size);
+remaining_size -= strlen(caps[cap]);
}
*value_name = s;
--
2.30.2
Re: [PATCH] sparc_attrs.c: Prevent buffer overflow in sparc_check_object_attribute
On Tue, Nov 5, 2024, at 9:25 AM, Anton Moryakov wrote: > Record Length Limit: We use strncat to add a line indicating the > available remaining_size. This prevents writing beyond the allocated > memory. > Remaining space update: remaining_size is updated after each entry to > ensure that row additions do not cause overflow. It looks to me like the maximum possible length of the concatenated strings (from a hardcoded array a few lines prior to the patch) and the length of the buffer are both statically known, and thus it's not actually possible for the code to overflow the buffer. This is an interesting test case for developing a static analyzer, but not an actual bug.
Re: [PATCH] sparc_attrs.c: Prevent buffer overflow in sparc_check_object_attribute
Hi, On Tue, Nov 05, 2024 at 11:58:19AM -0500, Serhei Makarov wrote: > On Tue, Nov 5, 2024, at 9:25 AM, Anton Moryakov wrote: > > Record Length Limit: We use strncat to add a line indicating the > > available remaining_size. This prevents writing beyond the allocated > > memory. > > Remaining space update: remaining_size is updated after each entry to > > ensure that row additions do not cause overflow. > > It looks to me like the maximum possible length of the concatenated > strings (from a hardcoded array a few lines prior to the patch) and > the length of the buffer are both statically known, and thus it's > not actually possible for the code to overflow the buffer. This is > an interesting test case for developing a static analyzer, but not > an actual bug. Or add a static_assert based on that knowledge as we discussed before when this "RASU JSC" issue came up: https://inbox.sourceware.org/elfutils-devel/20240702114611.ge29...@gnu.wildebeest.org/T Cheers, Mark
[PATCH] Обновить patches/0001-sparc_attrs.c-Prevent-buffer-overflow-in-sparc_check.patch
From: Моряков, Антон --- elfutils/backends/sparc_attrs.c | 5 - ...c_attrs.c-Prevent-buffer-overflow-in-sparc_check.patch | 8 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/elfutils/backends/sparc_attrs.c b/elfutils/backends/sparc_attrs.c index 974e8fb..a50b4f7 100644 --- a/elfutils/backends/sparc_attrs.c +++ b/elfutils/backends/sparc_attrs.c @@ -36,6 +36,9 @@ #define BACKEND sparc_ #include "libebl_CPU.h" +#define NAME_MAX_SIZE (32 * 17 + 32 + 1) +static_assert(NAME_MAX_SIZE == (32 * 17 + 32 + 1), "Buffer size for 'name' is insufficient"); + bool sparc_check_object_attribute (Ebl *ebl __attribute__ ((unused)), const char *vendor, int tag, uint64_t value, @@ -63,7 +66,7 @@ sparc_check_object_attribute (Ebl *ebl __attribute__ ((unused)), /* NAME should be big enough to hold any possible comma-separated list (no repetitions allowed) of attribute names from one of the arrays above. */ - static char name[32*17+32+1]; + static char name[NAME_MAX_SIZE]; name[0] = '\0'; if (!strcmp (vendor, "gnu")) diff --git a/patches/0001-sparc_attrs.c-Prevent-buffer-overflow-in-sparc_check.patch b/patches/0001-sparc_attrs.c-Prevent-buffer-overflow-in-sparc_check.patch index cb6a0c6..894ab4f 100644 --- a/patches/0001-sparc_attrs.c-Prevent-buffer-overflow-in-sparc_check.patch +++ b/patches/0001-sparc_attrs.c-Prevent-buffer-overflow-in-sparc_check.patch @@ -14,13 +14,13 @@ Found by RASU JSC. Signed-off-by: Anton Moryakov --- - elfutils/backends/sparc_attrs.c | 11 --- +backends/sparc_attrs.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) -diff --git a/elfutils/backends/sparc_attrs.c b/elfutils/backends/sparc_attrs.c +diff --git a/backends/sparc_attrs.c b/backends/sparc_attrs.c index 974e8fb..0ba17c8 100644 a/elfutils/backends/sparc_attrs.c -+++ b/elfutils/backends/sparc_attrs.c +--- a/backends/sparc_attrs.c b/backends/sparc_attrs.c @@ -87,12 +87,17 @@ sparc_check_object_attribute (Ebl *ebl __attribute__ ((unused)), } -- 2.30.2
[Bug tools/32253] FAIL: run-strip-reloc-self.sh with binutils master
https://sourceware.org/bugzilla/show_bug.cgi?id=32253 Aaron Merey changed: What|Removed |Added See Also||https://sourceware.org/bugz ||illa/show_bug.cgi?id=32341 -- You are receiving this mail because: You are on the CC list for the bug.
[PATCH] strip: Ignore --reloc-debug-sections-only for non-ET_REL files.
strip --reloc-debug-sections-only is expected to be a no-op for
non-ET_REL files. This was not enforced in the code. Sections
were copied over to a new output file and normally its contents
would be identical to the input file.
However the output file is not identical to a non-ET_REL input
file if the linker organized sections such that the indices of
SHF_ALLOC sections are not in a contigous group.
In this case strip will modify sections in order to keep all SHF_ALLOC
sections in a contiguous group.
Fix this by ignoring --reloc-debug-sections-only for non-ET_REL files.
https://sourceware.org/bugzilla/show_bug.cgi?id=32253
Signed-off-by: Aaron Merey
---
src/strip.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/strip.c b/src/strip.c
index 403e0f6f..3812fb17 100644
--- a/src/strip.c
+++ b/src/strip.c
@@ -1139,6 +1139,13 @@ handle_elf (int fd, Elf *elf, const char *prefix, const
char *fname,
if (reloc_debug_only)
{
+ if (ehdr->e_type != ET_REL)
+ {
+ /* Only ET_REL files should have debug relocations to remove. */
+ error (0, 0, _("Ignoring --reloc-debug-sections-only for " \
+"non-ET_REL file '%s'"), fname);
+ goto fail_close;
+ }
if (handle_debug_relocs (elf, ebl, newelf, ehdr, fname, shstrndx,
&lastsec_offset, &lastsec_size) != 0)
{
--
2.47.0
[Bug libelf/32311] elf_compress_gnu.c: gcc warns null-dereference with lto
https://sourceware.org/bugzilla/show_bug.cgi?id=32311 Mark Wielaard changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Assignee|unassigned at sourceware dot org |mark at klomp dot org Ever confirmed|0 |1 Last reconfirmed||2024-11-05 --- Comment #3 from Mark Wielaard --- Created attachment 15779 --> https://sourceware.org/bugzilla/attachment.cgi?id=15779&action=edit Use one getshdr call and a union We could help the compiler a little by just using one call to elf[32|64]_getshdr and reusing the result. Unfortunately I cannot replicate the warning/error even with -flto. Could you try this attached patch? -- You are receiving this mail because: You are on the CC list for the bug.
