mirror of
https://https.git.savannah.gnu.org/git/patch.git
synced 2026-01-27 01:44:34 +00:00
Ignore dangerous filenames instead of failing immediately
* src/pch.c (name_is_valid): New function. (intuit_diff_type, best_name): Use name_is_valid() here. (strip_leading_slashes): Remove name validation tests from here. * tests/bad-filenames: Add more tests for covering more of the file name guessing corner cases in intuit_diff_type(), update the existing tests. * NEWS: Update.
This commit is contained in:
Andreas Gruenbacher
parent
dcfb493578
commit
f663762bf0
10
ChangeLog
10
ChangeLog
@ -1,3 +1,13 @@
|
||||
2011-02-16 Andreas Gruenbacher <agruen@linbit.com>
|
||||
|
||||
* src/pch.c (name_is_valid): New function.
|
||||
(intuit_diff_type, best_name): Use name_is_valid() here.
|
||||
(strip_leading_slashes): Remove name validation tests from here.
|
||||
* tests/bad-filenames: Add more tests for covering more of the
|
||||
file name guessing corner cases in intuit_diff_type(), update the
|
||||
existing tests.
|
||||
* NEWS: Update.
|
||||
|
||||
2011-02-15 Andreas Gruenbacher <agruen@linbit.com>
|
||||
|
||||
* src/patch.c (main): Fix use of initialized outst and add an
|
||||
|
||||
2
NEWS
2
NEWS
@ -1,4 +1,4 @@
|
||||
* patch now rejects a destination file name that is absolute or that contains
|
||||
* Patch now ignores destination file names that are absolute or that contain
|
||||
a component of "..". This addresses CVE-2010-4651,
|
||||
* Support for most features of the "diff --git" format: renames and copies,
|
||||
permission changes, symlink diffs. Caveats:
|
||||
|
||||
28
src/pch.c
28
src/pch.c
@ -376,6 +376,31 @@ skip_hex_digits (char const *str)
|
||||
return s == str ? NULL : s;
|
||||
}
|
||||
|
||||
static bool
|
||||
name_is_valid (char const *name)
|
||||
{
|
||||
const char *n = name;
|
||||
|
||||
if (IS_ABSOLUTE_FILE_NAME (name))
|
||||
{
|
||||
say ("Ignoring potentially dangerous file name %s\n", quotearg (name));
|
||||
return false;
|
||||
}
|
||||
for (n = name; *n; )
|
||||
{
|
||||
if (*n == '.' && *++n == '.' && ( ! *++n || ISSLASH (*n)))
|
||||
{
|
||||
say ("Ignoring potentially dangerous file name %s\n", quotearg (name));
|
||||
return false;
|
||||
}
|
||||
while (*n && ! ISSLASH (*n))
|
||||
n++;
|
||||
while (ISSLASH (*n))
|
||||
n++;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/* Determine what kind of diff is in the remaining part of the patch file. */
|
||||
|
||||
static enum diff
|
||||
@ -829,7 +854,7 @@ intuit_diff_type (bool need_header, mode_t *p_file_type)
|
||||
else
|
||||
{
|
||||
stat_errno[i] = 0;
|
||||
if (posixly_correct)
|
||||
if (posixly_correct && name_is_valid (p_name[i]))
|
||||
break;
|
||||
}
|
||||
i0 = i;
|
||||
@ -1002,6 +1027,7 @@ best_name (char *const *name, int const *ignore)
|
||||
/* Of those, take the first name. */
|
||||
for (i = OLD; i <= INDEX; i++)
|
||||
if (name[i] && !ignore[i]
|
||||
&& name_is_valid (name[i])
|
||||
&& components[i] == components_min
|
||||
&& basename_len[i] == basename_len_min
|
||||
&& len[i] == len_min)
|
||||
|
||||
11
src/util.c
11
src/util.c
@ -1415,17 +1415,6 @@ strip_leading_slashes (char *name, int strip_leading)
|
||||
n = p+1;
|
||||
}
|
||||
}
|
||||
if (IS_ABSOLUTE_FILE_NAME (n))
|
||||
fatal ("rejecting absolute file name: %s", quotearg (n));
|
||||
for (p = n; *p; )
|
||||
{
|
||||
if (*p == '.' && *++p == '.' && ( ! *++p || ISSLASH (*p)))
|
||||
fatal ("rejecting file name with \"..\" component: %s", quotearg (n));
|
||||
while (*p && ! ISSLASH (*p))
|
||||
p++;
|
||||
while (ISSLASH (*p))
|
||||
p++;
|
||||
}
|
||||
if ((strip_leading < 0 || s <= 0) && *n)
|
||||
{
|
||||
memmove (name, n, strlen (n) + 1);
|
||||
|
||||
@ -7,6 +7,7 @@
|
||||
. $srcdir/test-lib.sh
|
||||
|
||||
use_local_patch
|
||||
use_tmpdir
|
||||
|
||||
# ================================================================
|
||||
|
||||
@ -23,27 +24,93 @@ EOF
|
||||
# Ensure that patch rejects an output file name that is absolute
|
||||
# or that contains a ".." component.
|
||||
|
||||
check 'emit_patch /absolute/path | patch -p0; echo status: $?' <<EOF
|
||||
$PATCH: **** rejecting absolute file name: /absolute/path
|
||||
status: 2
|
||||
check 'emit_patch ../z | patch -f -p1 --dry-run || echo status: $?' <<EOF
|
||||
patching file z
|
||||
EOF
|
||||
|
||||
check 'emit_patch a/../z | patch -p0; echo status: $?' <<EOF
|
||||
$PATCH: **** rejecting file name with ".." component: a/../z
|
||||
status: 2
|
||||
check 'emit_patch /absolute/path | patch -f -p0 --dry-run || echo status: $?' <<EOF
|
||||
Ignoring potentially dangerous file name /absolute/path
|
||||
can't find file to patch at input line 3
|
||||
Perhaps you used the wrong -p or --strip option?
|
||||
The text leading up to this was:
|
||||
--------------------------
|
||||
|--- /dev/null
|
||||
|+++ /absolute/path
|
||||
--------------------------
|
||||
No file to patch. Skipping patch.
|
||||
1 out of 1 hunk ignored
|
||||
status: 1
|
||||
EOF
|
||||
|
||||
check 'emit_patch a/../z | patch -p1; echo status: $?' <<EOF
|
||||
$PATCH: **** rejecting file name with ".." component: ../z
|
||||
status: 2
|
||||
check 'emit_patch a/../z | patch -f -p0 --dry-run || echo status: $?' <<EOF
|
||||
Ignoring potentially dangerous file name a/../z
|
||||
can't find file to patch at input line 3
|
||||
Perhaps you used the wrong -p or --strip option?
|
||||
The text leading up to this was:
|
||||
--------------------------
|
||||
|--- /dev/null
|
||||
|+++ a/../z
|
||||
--------------------------
|
||||
No file to patch. Skipping patch.
|
||||
1 out of 1 hunk ignored
|
||||
status: 1
|
||||
EOF
|
||||
|
||||
check 'emit_patch a/.. | patch -p0; echo status: $?' <<EOF
|
||||
$PATCH: **** rejecting file name with ".." component: a/..
|
||||
status: 2
|
||||
check 'emit_patch a/../z | patch -f -p1 --dry-run || echo status: $?' <<EOF
|
||||
Ignoring potentially dangerous file name ../z
|
||||
can't find file to patch at input line 3
|
||||
Perhaps you used the wrong -p or --strip option?
|
||||
The text leading up to this was:
|
||||
--------------------------
|
||||
|--- /dev/null
|
||||
|+++ a/../z
|
||||
--------------------------
|
||||
No file to patch. Skipping patch.
|
||||
1 out of 1 hunk ignored
|
||||
status: 1
|
||||
EOF
|
||||
|
||||
check 'emit_patch ../z | patch -p0; echo status: $?' <<EOF
|
||||
$PATCH: **** rejecting file name with ".." component: ../z
|
||||
status: 2
|
||||
check 'emit_patch ../z | patch -f -p0 --dry-run || echo status: $?' <<EOF
|
||||
Ignoring potentially dangerous file name ../z
|
||||
can't find file to patch at input line 3
|
||||
Perhaps you used the wrong -p or --strip option?
|
||||
The text leading up to this was:
|
||||
--------------------------
|
||||
|--- /dev/null
|
||||
|+++ ../z
|
||||
--------------------------
|
||||
No file to patch. Skipping patch.
|
||||
1 out of 1 hunk ignored
|
||||
status: 1
|
||||
EOF
|
||||
|
||||
cat > d.diff <<EOF
|
||||
--- f
|
||||
+++ ../g
|
||||
@@ -1 +1 @@
|
||||
-1
|
||||
+1
|
||||
EOF
|
||||
|
||||
check 'patch -f -p0 --dry-run < d.diff || echo status: $?' <<EOF
|
||||
can't find file to patch at input line 3
|
||||
Perhaps you used the wrong -p or --strip option?
|
||||
The text leading up to this was:
|
||||
--------------------------
|
||||
|--- f
|
||||
|+++ ../g
|
||||
--------------------------
|
||||
No file to patch. Skipping patch.
|
||||
1 out of 1 hunk ignored
|
||||
status: 1
|
||||
EOF
|
||||
|
||||
echo 1 > f
|
||||
check 'patch -f -p0 --dry-run < d.diff || echo status: $?' <<EOF
|
||||
patching file f
|
||||
EOF
|
||||
|
||||
echo 1 > g
|
||||
check 'patch -f -p1 --dry-run < d.diff || echo status: $?' <<EOF
|
||||
patching file g
|
||||
EOF
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user