github.com/Interlisp.maiko
1
0
Fork 0
mirror of https://github.com/Interlisp/maiko.git synced 2026-01-28 20:41:30 +00:00
Code Issues Projects Releases Wiki Activity

Command line: fix potential buffer overruns in argument/env variable handling (#375)

Browse Source 
* Fix buffer overrun vulnerability: use strncpy

read_Xoption uses a char buffer defined in main.c with length
MAXPATHLEN, aka PATH_MAX in POSIX. Unfortunately it was using strcpy to
copy from the command-line arguments (via argv) and the environment (via
getenv) without any bounds checking whatsoever. This could very easily
cause an overflow.

It's unlikely that a user will want to provide a path longer than
PATH_MAX-1 (a generous 1023 bytes on my machine). If they try, we should
stop them from causing any damage.

* Use strlcpy instead of strncpy

Thanks to Nick Briggs for the suggestion. It would be best to use
sizeof(sysout_name) instead of hardcoding a reference to the PATH_MAX
constant, but unfortunately sysout_name is an extern in xrdopt.c and so
the compiler doesn't know its size. I don't want to mess with that
coupling in this commit, because I assume there was a reason for doing
it that way rather than putting sysout_name in a header; I'll keep the
scope of the changes here small.

* Revert "Use strlcpy instead of strncpy"

Ah. This is not great. Turns out strlcpy is a nonstandard BSD extension
with its own set of problems
[https://en.wikibooks.org/wiki/C_Programming/C_Reference/nonstandard/strlcpy]
that we may be best served by avoiding. On Linux, it's only accessible
through libbsd, and we have no other reason (as far as I can tell) to
require that. Unless we want to provide our own strlcpy implementation,
we should stick with strncpy. It's far safer than what was there before
and doesn't present any edge cases in this scenario that are apparent to
me.
This commit is contained in:
Abe Jellinek
2021-04-05 14:43:37 -07:00
committed by GitHub
parent 081e2288ff
commit 5ce292ed32
1 changed files with 4 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
src/xrdopt.c
View File

@@ -17,6 +17,7 @@
#include <sys/file.h>
#include <sys/time.h>
#include <unistd.h>
#include <limits.h>
#ifdef MAIKO_ENABLE_ETHERNET
#ifndef USE_DLPI
#include <net/nit.h> /* needed for Ethernet stuff below */
@@ -173,11 +174,11 @@ void read_Xoption(int *argc, char *argv[])
sysout_name[0] = '\0';
if (*argc == 2) /* There was probably a sysoutarg */
{
(void)strcpy(sysout_name, argv[1]);
(void)strncpy(sysout_name, argv[1], PATH_MAX - 1);
} else if ((envname = getenv("LDESRCESYSOUT")) != NULL) {
strcpy(sysout_name, envname);
strncpy(sysout_name, envname, PATH_MAX - 1);
} else if ((envname = getenv("LDESOURCESYSOUT")) != NULL)
strcpy(sysout_name, envname);
strncpy(sysout_name, envname, PATH_MAX - 1);
else {
envname = getenv("HOME");
(void)strcat(sysout_name, envname);