From 3904769ba8ff4c4348fcadcf07fdc7b6a1ff25e4 Mon Sep 17 00:00:00 2001 From: Alexander Leidinger Date: Sun, 31 Jul 2005 10:28:35 +0000 Subject: [PATCH] Add bounds checking to the setenv part of the kernel environment. This has no security implications since only root is allowed to use kenv(1) (and corrupt the kernel memory after adding too much variables previous to this commit). This is based upon the PR [1] mentioned below, but extended to check both bounds (in case of an overflow of the counting variable) and to comply to the style of the function. An overflow of the counting variable shouldn't happen after adding the check for the upper bound, but better safe than sorry (in case some other function in the kernel overwrites random memory). An interested soul may want to add a printf to notify root in case the bounds are hit. Also allocate KENV_SIZE+1 entries (the array is NULL-terminated), since the comment for KENV_SIZE says it's the maximum number of environment strings. [2] PR: 83687 [1] Submitted by: Harry Coin [1] Submitted by: Ariff Abdullah [2] --- sys/kern/kern_environment.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/sys/kern/kern_environment.c b/sys/kern/kern_environment.c index 8c488644dc78..4fd84ae5310d 100644 --- a/sys/kern/kern_environment.c +++ b/sys/kern/kern_environment.c @@ -205,7 +205,8 @@ init_dynamic_kenv(void *data __unused) char *cp; int len, i; - kenvp = malloc(KENV_SIZE * sizeof(char *), M_KENV, M_WAITOK | M_ZERO); + kenvp = malloc((KENV_SIZE + 1) * sizeof(char *), M_KENV, + M_WAITOK | M_ZERO); i = 0; for (cp = kern_envp; cp != NULL; cp = kernenv_next(cp)) { len = strlen(cp) + 1; @@ -349,6 +350,14 @@ setenv(const char *name, const char *value) /* We add the option if it wasn't found */ for (i = 0; (cp = kenvp[i]) != NULL; i++) ; + + /* Bounds checking */ + if (i < 0 || i >= KENV_SIZE) { + free(buf, M_KENV); + sx_xunlock(&kenv_lock); + return (-1); + } + kenvp[i] = buf; kenvp[i + 1] = NULL; sx_xunlock(&kenv_lock);