lkml.org 
[lkml]   [2019]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH v2] exec: load_script: Allow interpreter argument truncation
While we want to make sure the kernel doesn't attempt to execute a
truncated interpreter path, we must allow the interpreter arguments to
be truncated. Perl, for example, will re-read the script itself to parse
arguments correctly.

This documents the parsing steps, and will fail to exec if the string was
truncated with neither an end-of-line nor any trailing whitespace.

Reported-and-Tested-by: Samuel Dionne-Riel <samuel@dionne-riel.com>
Fixes: 8099b047ecc4 ("exec: load_script: don't blindly truncate shebang string")
Signed-off-by: Kees Cook <keescook@chromium.org>
---
v2:
- fix 1-byte-too-early-bail-out in truncation detection (Oleg)
- add Samuel's "tested" tag
---
fs/binfmt_script.c | 37 +++++++++++++++++++++++++++++++------
1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index d0078cbb718b..fedcbf3e1f1c 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -20,6 +20,7 @@ static int load_script(struct linux_binprm *bprm)
char *cp;
struct file *file;
int retval;
+ bool truncated = false, end_of_interp = false;

if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!'))
return -ENOEXEC;
@@ -42,32 +43,56 @@ static int load_script(struct linux_binprm *bprm)
fput(bprm->file);
bprm->file = NULL;

+ /*
+ * Truncating interpreter arguments is okay: the interpreter
+ * can re-read the script to parse them on its own. Truncating
+ * the interpreter path itself, though, is bad. Note truncation
+ * here, and check for either newline or start of arguments
+ * below.
+ */
for (cp = bprm->buf+2;; cp++) {
- if (cp >= bprm->buf + BINPRM_BUF_SIZE)
- return -ENOEXEC;
- if (!*cp || (*cp == '\n'))
+ if (!*cp || (*cp == '\n')) {
+ end_of_interp = true;
break;
+ }
+ if (cp == bprm->buf + BINPRM_BUF_SIZE - 1) {
+ truncated = true;
+ break;
+ }
}
*cp = '\0';

+ /* Truncate trailing whitespace */
while (cp > bprm->buf) {
cp--;
- if ((*cp == ' ') || (*cp == '\t'))
+ if ((*cp == ' ') || (*cp == '\t')) {
+ end_of_interp = true;
*cp = '\0';
- else
+ } else
break;
}
+ /* Skip leading whitespace */
for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++);
if (*cp == '\0')
return -ENOEXEC; /* No interpreter name found */
i_name = cp;
i_arg = NULL;
+ /*
+ * Skip until end of string or finding whitespace which
+ * signals the start of interpreter arguments.
+ */
for ( ; *cp && (*cp != ' ') && (*cp != '\t'); cp++)
/* nothing */ ;
- while ((*cp == ' ') || (*cp == '\t'))
+ /* Truncate and skip any whitespace in front of arguments */
+ while ((*cp == ' ') || (*cp == '\t')) {
+ end_of_interp = true;
*cp++ = '\0';
+ }
if (*cp)
i_arg = cp;
+ /* Fail exec if the name of the interpreter was cut off. */
+ if (truncated && !end_of_interp)
+ return -ENOEXEC;
/*
* OK, we've parsed out the interpreter name and
* (optional) argument.
--
2.17.1

--
Kees Cook

\
 
 \ /
  Last update: 2019-02-14 17:44    [W:1.255 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site