Taking strlen of buffers copied from userspace

From: Artem Frolov
Date: Tue Mar 15 2005 - 13:40:39 EST


Hello,

I am in the process of testing static defect analyzer on a Linux
kernel source code (see disclosure below).

I found some potential array bounds violations. The pattern is as
follows: bytes are copied from the user space and then buffer is
accessed on index strlen(buf)-1. This is a defect if user data start
from 0. So the question is: can we make any assumptions what data may
be received from the user or it could be arbitrary?

For example, in ./drivers/block/cciss.c, function cciss_proc_write
(line numbers are taken form 2.6.11.3):
....
293 if (count > sizeof(cmd)-1) return -EINVAL;
294 if (copy_from_user(cmd, buffer, count)) return -EFAULT;
295 cmd[count] = '\0';
296 len = strlen(cmd); // above 3 lines ensure safety
297 if (cmd[len-1] == '\n')
298 cmd[--len] = '\0';
.....

Another example is arch/i386/kernel/cpu/mtrr/if.c, function mtrr_write:
....
107 if (copy_from_user(line, buf, len - 1))
108 return -EFAULT;
109 ptr = line + strlen(line) - 1;
110 if (*ptr == '\n')
111 *ptr = '\0';
....


Full disclosure: I am working for Klocwork (http://www.klocwork.com/),
which is a vendor of commercial closed-source proprietary products,
static analyzer for C/C++ is part of its products

Best regards
--
Artem Frolov
Senior software engineer
Klocwork inc
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/