From 028c1900ad0994de033237eeb7afc93fa0a1a75c Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Fri, 2 Jun 2017 15:40:00 -0400
Subject: [PATCH 15/22] Fix some plausible buffer overruns.
Covscan noticed these:
Error: STRING_OVERFLOW (CWE-120): [#def28]
libsmbios-2.3.3/src/libsmbios_c/smi/smi_linux.c:128: fixed_size_dest: You might overrun the 256-character fixed-size string "fn" by copying "sysfs_basedir" without checking the length.
Error: STRING_OVERFLOW (CWE-120): [#def29]
libsmbios-2.3.3/src/libsmbios_c/smi/smi_linux.c:129: fixed_size_dest: You might overrun the 256-character fixed-size string "fn" by copying "smi_data_fn" without checking the length.
Error: STRING_OVERFLOW (CWE-120): [#def30]
libsmbios-2.3.3/src/libsmbios_c/smi/smi_linux.c:170: fixed_size_dest: You might overrun the 256-character fixed-size string "fn" by copying "sysfs_basedir" without checking the length.
Error: STRING_OVERFLOW (CWE-120): [#def31]
libsmbios-2.3.3/src/libsmbios_c/smi/smi_linux.c:171: fixed_size_dest: You might overrun the 256-character fixed-size string "fn" by copying "smi_data_fn" without checking the length.
Error: STRING_OVERFLOW (CWE-120): [#def32]
libsmbios-2.3.3/src/libsmbios_c/smi/smi_linux.c:188: fixed_size_dest: You might overrun the 256-character fixed-size string "fn" by copying "sysfs_basedir" without checking the length.
Error: STRING_OVERFLOW (CWE-120): [#def33]
libsmbios-2.3.3/src/libsmbios_c/smi/smi_linux.c:189: fixed_size_dest: You might overrun the 256-character fixed-size string "fn" by copying "smi_request_fn" without checking the length.
I don't know how big they can actually be - it depends on what
sysfs_basedir winds up being, which I imagine is smallish so long as
sysfs is on /sys, but it doesn't need to be - but I think covscan is
right, if impractical.
This patch replaces all of these with an allocate_path(a, b) call that
allocates the correct amount of space on the stack and copies the data
in.
Signed-off-by: Peter Jones <pjones@redhat.com>
---
src/libsmbios_c/smi/smi_linux.c | 75 +++++++++++++++++++++++++++--------------
1 file changed, 50 insertions(+), 25 deletions(-)
diff --git a/src/libsmbios_c/smi/smi_linux.c b/src/libsmbios_c/smi/smi_linux.c
index 46ad888282d..f7628a1800c 100644
--- a/src/libsmbios_c/smi/smi_linux.c
+++ b/src/libsmbios_c/smi/smi_linux.c
@@ -22,6 +22,7 @@
#include "smbios_c/compat.h"
// system
+#include <alloca.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -53,9 +54,22 @@ void set_basedir(const char *newdir)
sysfs_basedir = newdir;
}
+#define allocate_path(a, b) ( \
+ { \
+ char *fn_; \
+ size_t sz_ = strlen(a) + strlen(b) + 1; \
+ fn_ = alloca(sz_); \
+ if (fn_) \
+ { \
+ strcpy(fn_, (a)); \
+ strcat(fn_, (b)); \
+ } \
+ fn_; \
+ })
+
u32 __hidden get_phys_buf_addr()
{
- char fn[bufsize] = {0,};
+ char *fn = NULL;
FILE *fd = 0;
u32 physaddr = 0;
char linebuf[bufsize] = {0,};
@@ -63,8 +77,10 @@ u32 __hidden get_phys_buf_addr()
fnprintf("\n");
- strcat(fn, sysfs_basedir);
- strcat(fn, smi_data_buf_phys_addr_fn);
+ fn = allocate_path(sysfs_basedir, smi_data_buf_phys_addr_fn);
+ if (!fn)
+ goto out;
+
fd = fopen(fn, "rb");
if (!fd)
goto out;
@@ -79,7 +95,8 @@ u32 __hidden get_phys_buf_addr()
physaddr = strtoll(linebuf, NULL, 16);
out_close:
- fclose(fd);
+ if (fd)
+ fclose(fd);
fflush(NULL);
out:
@@ -89,15 +106,17 @@ out:
// returns physaddr
u32 __hidden set_phys_buf_size(u32 newsize)
{
- char fn[bufsize] = {0,};
+ char *fn;
FILE *fd = NULL;
char linebuf[bufsize] = {0,};
u32 phys_buf_addr=0;
fnprintf("\n");
- strcat(fn, sysfs_basedir);
- strcat(fn, smi_data_buf_size_fn);
+ fn = allocate_path(sysfs_basedir, smi_data_buf_size_fn);
+ if (!fn)
+ goto out;
+
fd = fopen(fn, "w+b");
if (!fd)
goto out;
@@ -107,26 +126,26 @@ u32 __hidden set_phys_buf_size(u32 newsize)
if (recs != 1)
goto out;
- fclose(fd);
-
- fflush(NULL);
-
phys_buf_addr = get_phys_buf_addr();
- goto out;
out:
+ if (fd)
+ fclose(fd);
+ fflush(NULL);
+
return phys_buf_addr;
}
void __hidden write_smi_data(u8 *buffer, size_t size)
{
- char fn[bufsize] = {0,};
+ char *fn;
FILE *fd = 0;
fnprintf("\n");
- strcat(fn, sysfs_basedir);
- strcat(fn, smi_data_fn);
+ fn = allocate_path(sysfs_basedir, smi_data_fn);
+ if (!fn)
+ goto out;
fnprintf("open data file: '%s'\n", fn);
@@ -162,13 +181,15 @@ out:
void __hidden get_smi_results(u8 *buffer, size_t size)
{
- char fn[bufsize] = {0,};
+ char *fn;
FILE *fd = 0;
fnprintf("\n");
- strcat(fn, sysfs_basedir);
- strcat(fn, smi_data_fn);
+ fn = allocate_path(sysfs_basedir, smi_data_fn);
+ if (!fn)
+ goto out;
+
fd = fopen(fn, "rb");
if (!fd)
goto out;
@@ -182,17 +203,21 @@ out:
FILE *open_request_file()
{
- char fn[bufsize] = {0,};
+ char *fn;
FILE *fd = 0;
int ret;
- strcat(fn, sysfs_basedir);
- strcat(fn, smi_request_fn);
+
+ fn = allocate_path(sysfs_basedir, smi_request_fn);
+ if (!fn)
+ return NULL;
+
fnprintf("open request file: '%s'\n", fn);
fd = fopen(fn, "wb");
- if(fd)
- flock( fileno(fd), LOCK_EX );
- if(fd)
- ret = fwrite("0", 1, 1, fd);
+ if (!fd)
+ return NULL;
+
+ flock( fileno(fd), LOCK_EX );
+ ret = fwrite("0", 1, 1, fd);
fnprintf("got fd for request file: %p\n", fd);
UNREFERENCED_PARAMETER(ret);
--
2.14.3