linux-kernel - Re: [PATCH] execve.2: execve also returns E2BIG if a string is too long

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231011151126.754612-1-mattlloydhouse@gmail.com>
Date:   Wed, 11 Oct 2023 11:11:24 -0400
From:   Matthew House <mattlloydhouse@...il.com>
To:     Alejandro Colomar <alx@...nel.org>
Cc:     Rik van Riel <riel@...riel.com>, linux-man@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>, kernel-team@...a.com,
        Eric Biederman <ebiederm@...ssion.com>
Subject: Re: [PATCH] execve.2: execve also returns E2BIG if a string is too long

On Wed, Oct 11, 2023 at 10:47 AM Alejandro Colomar <alx@...nel.org> wrote:
> On Wed, Oct 11, 2023 at 09:44:29AM -0400, Matthew House wrote:
> > To expand on this, there are basically two separate byte limits in
> > fs/exec.c, one for each individual argv/envp string, and another for all
> > strings and all pointers to them as a whole. To put the whole thing in
> > pseudocode, the checks work effectively like this, assuming I haven't made
> > any errors:
> >
> > int argc, envc;
> > unsigned long bytes, limit;
> >
> > /* assume that argv has already been adjusted to add an empty argv[0] */
> > argc = 0, envc = 0, bytes = 0;
> > for (char **a = argv; *a != NULL; a++, argc++) {
> >     if (strlen(*a) >= MAX_ARG_STRLEN)
>
> Are you sure this is >= and not > ?

Yes. In general, the kernel's string limits tend to include the trailing
null byte. There are two places where this limit is enforced, one for the
pathname (or full pathname for execveat) and the other for the argv/envp
strings. The pathname is handled by copy_string_kernel():

	int len = strnlen(arg, MAX_ARG_STRLEN) + 1 /* terminating NUL */;

	if (len == 0)
		return -EFAULT;
	if (!valid_arg_len(bprm, len))
		return -E2BIG;

where valid_arg_len(bprm, len) is just (len <= MAX_ARG_STRLEN). Here,
strnlen() has the same behavior as the ordinary libc strnlen(3),
effectively returning min(strlen(arg), MAX_ARG_STRLEN). Thus, the check
succeeds iff strlen(arg) + 1 <= MAX_ARG_STRLEN, or equivalently, iff
strlen(arg) < MAX_ARG_STRLEN.

Next, each of the environment and argument strings is handled by
copy_strings():

		len = strnlen_user(str, MAX_ARG_STRLEN);
		if (!len)
			goto out;

		ret = -E2BIG;
		if (!valid_arg_len(bprm, len))
			goto out;

The strnlen_user() function, per its documentation, is explicitly inclusive
of the trailing null byte:

 * Returns the size of the string INCLUDING the terminating NUL.
 * If the string is too long, returns a number larger than @count. User
 * has to check the return value against "> count".
 * On exception (or invalid count), returns 0.

Thus, the check succeeds iff the size including the null byte is
<= MAX_ARG_STRLEN, i.e., iff strlen(arg) + 1 <= MAX_ARG_STRLEN, or
strlen(arg) < MAX_ARG_STRLEN.

Matthew House