[PATCH] tomoyo: reject short exec.envp[] names before suffix checks

[PATCH] tomoyo: reject short exec.envp[] names before suffix checks

[Pengpeng Hou]

tomoyo_parse_envp() assumes that the left-hand side still ends with the
closing '"' and ']' from an exec.envp["..."] condition and immediately
backs up from strlen(left) - 1 to verify that suffix.

If policy input leaves an empty or one-byte string here, the parser
reads before the start of the token while checking for the suffix.

Reject left-hand strings that are too short to contain the required '"]'
terminator before dereferencing the trailing characters.

Fixes: 5b636857fee6 ("TOMOYO: Allow using argv[]/envp[] of execve() as conditions.")
Cc: stable@vger.kernel.org

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 security/tomoyo/condition.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/security/tomoyo/condition.c b/security/tomoyo/condition.c
index f8bcc083bb0d..1fa8343df4b3 100644
--- a/security/tomoyo/condition.c
+++ b/security/tomoyo/condition.c
@@ -320,7 +320,13 @@ static bool tomoyo_parse_envp(char *left, char *right,
 {
 	const struct tomoyo_path_info *name;
 	const struct tomoyo_path_info *value;
-	char *cp = left + strlen(left) - 1;
+	size_t len = strlen(left);
+	char *cp;
+
+	if (len < 2)
+		goto out;
+
+	cp = left + len - 1;
 
 	if (*cp-- != ']' || *cp != '"')
 		goto out;
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] tomoyo: reject short exec.envp[] names before suffix checks

[Tetsuo Handa]

Thank you for a patch, but I don't think we need to apply this patch.

The caller is

    if (!strncmp(left_word, "exec.envp[\"", 11)) {
        (...snipped...)
        if (!tomoyo_parse_envp(left_word + 11, right_word, envp++)) goto out;
        (...snipped...)
    }

where the left-hand string is guaranteed to be safely dereferenced.

Re: [PATCH] tomoyo: reject short exec.envp[] names before suffix checks

[Pengpeng Hou]

Hi Tetsuo,

Thanks for the explanation.

Agreed, I missed that the left-hand string is already guaranteed to be
safely dereferenced at that call site. I'll drop this patch.

Thanks,
Pengpeng