Re: [RFC][PATCH] Privilege dropping security module

[daw@cs.berkeley.edu (David Wagner)]

Andy Spencer  wrote:
>static ssize_t dpriv_stage_write(struct file *filp, const char *ubuffer,
>		size_t length, loff_t *off)
>{
>	struct file *file;
>	int err, rval, perm;
>	char *kbuffer, *perm_str, *path_str;
>	int perm_start, perm_end, path_start;
>
>	if (!(kbuffer = kzalloc(length+1, GFP_KERNEL)))
>		return -ENOMEM;
>
>	if (copy_from_user(kbuffer, ubuffer, length))
>		goto fail_fault;

Can 'length+1' overflow?
(Can the caller arrange to pass MAX_SIZE_T as the length parameter?
If yes, that's a vulnerability.)
I haven't checked how dpriv_stage_write() is called, to see whether
this is possible.

>	/* Parse input */
>	path_start = -1;
>	sscanf(kbuffer, " %n%*s%n %n", &perm_start, &perm_end, &path_start);
>	if (path_start == -1)
>		goto fail_inval;
>	perm_str = kbuffer+perm_start;
>	kbuffer[perm_end] = '\0';
>	path_str = kbuffer+path_start;

What if kbuffer isn't '\0'-terminated?  Won't this read past the end
of kbuffer?

Are you certain that perm_end and path_start will be within bounds?
If the user supplies a sufficiently large string (more than MAX_INT
characters long), could perm_end or path_start be negative?

>	rval = length;

Converts size_t to ssize_t.