From 593d7997706e523069e0f1c1a3330e9201aab442 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Fri, 6 Apr 2012 14:43:33 +0200 Subject: [PATCH] Fix use-after-free bug in name_is_valid() * src/common.h (ARRAY_SIZE): New macro. * src/pch.c (invalid_names): New global variable for remembering bad names. (intuit_diff_type): Reset invalid_names for each new patch; the names from the previous patch have already been freed. (name_is_valid): Use invalid_names. Make the code "safer" and avoid duplication. --- src/common.h | 1 + src/pch.c | 52 +++++++++++++++++++++++++++++++--------------------- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/common.h b/src/common.h index d91f696..bc93e59 100644 --- a/src/common.h +++ b/src/common.h @@ -62,6 +62,7 @@ #define strEQ(s1,s2) (!strcmp(s1, s2)) #define strnEQ(s1,s2,l) (!strncmp(s1, s2, l)) +#define ARRAY_SIZE(a) (sizeof (a) / sizeof ((a)[0])) /* typedefs */ diff --git a/src/pch.c b/src/pch.c index 6b0605e..a07f291 100644 --- a/src/pch.c +++ b/src/pch.c @@ -42,6 +42,7 @@ static int p_says_nonexistent[2]; /* [0] for old file, [1] for new: 2 for nonexistent */ static int p_rfc934_nesting; /* RFC 934 nesting level */ static char *p_name[3]; /* filenames in patch headers */ +static char const *invalid_names[2]; bool p_copy[2]; /* Does this patch create a copy? */ bool p_rename[2]; /* Does this patch rename a file? */ static char *p_timestr[2]; /* timestamps as strings */ @@ -379,34 +380,41 @@ skip_hex_digits (char const *str) static bool name_is_valid (char const *name) { - static char const *bad[2]; char const *n; + int i; + bool is_valid = true; - if (bad[0] && ! strcmp (bad[0], name)) - return false; - if (bad[1] && ! strcmp (bad[1], name)) - return false; + for (i = 0; i < ARRAY_SIZE (invalid_names); i++) + { + if (! invalid_names[i]) + break; + if (! strcmp (invalid_names[i], name)) + return false; + } if (IS_ABSOLUTE_FILE_NAME (name)) + is_valid = false; + else + for (n = name; *n; ) + { + if (*n == '.' && *++n == '.' && ( ! *++n || ISSLASH (*n))) + { + is_valid = false; + break; + } + while (*n && ! ISSLASH (*n)) + n++; + while (ISSLASH (*n)) + n++; + } + + if (! is_valid) { say ("Ignoring potentially dangerous file name %s\n", quotearg (name)); - bad[!! bad[0]] = name; - return false; - } - for (n = name; *n; ) - { - if (*n == '.' && *++n == '.' && ( ! *++n || ISSLASH (*n))) - { - say ("Ignoring potentially dangerous file name %s\n", quotearg (name)); - bad[!! bad[0]] = name; - return false; - } - while (*n && ! ISSLASH (*n)) - n++; - while (ISSLASH (*n)) - n++; + if (i < ARRAY_SIZE (invalid_names)) + invalid_names[i] = name; } - return true; + return is_valid; } /* Determine what kind of diff is in the remaining part of the patch file. */ @@ -434,6 +442,8 @@ intuit_diff_type (bool need_header, mode_t *p_file_type) free (p_name[i]); p_name[i] = 0; } + for (i = 0; i < ARRAY_SIZE (invalid_names); i++) + invalid_names[i] = NULL; for (i = OLD; i <= NEW; i++) if (p_timestr[i]) { -- 1.7.7.6