diff options
author | Andreas Gruenbacher <agruen@linbit.com> | 2012-04-06 14:43:33 +0200 |
---|---|---|
committer | Andreas Gruenbacher <agruen@linbit.com> | 2012-04-06 20:29:47 +0200 |
commit | ed081db4b169495f75c5b5274bf547e242c58712 (patch) | |
tree | c9e469a4453b7a548b9c74fb0cd56e885466cfc0 | |
parent | a8699f834be673d69b30909d9e5d53981d4cb00e (diff) | |
download | patch-ed081db4b169495f75c5b5274bf547e242c58712.tar.gz |
Fix use-after-free bug in name_is_valid()
Reported by Steffen Sledz in
https://bugzilla.novell.com/show_bug.cgi?id=755136 via Jean Delvare.
Bug introduced in commit v2.6.1-115-ge0f7075; fixed with help from Jim
Meyering <meyering@redhat.com>.
* 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.
-rw-r--r-- | src/common.h | 1 | ||||
-rw-r--r-- | 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 */ @@ -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]) { |