[PATCH v2] exec: load_script: Allow interpreter argument truncation

[Kees Cook]

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 <sam...@dionne-riel.com>
Fixes: 8099b047ecc4 ("exec: load_script: don't blindly truncate shebang string")
Signed-off-by: Kees Cook <keesc...@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