[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [bug-patch] [PATCH] do not validate target name when it is specified
From: |
Jim Meyering |
Subject: |
Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line |
Date: |
Wed, 16 Feb 2011 15:27:42 +0100 |
Andreas Gruenbacher wrote:
> On Monday 14 February 2011 10:34:59 Jim Meyering wrote:
>> Andreas Gruenbacher wrote:
>>
>> > On Monday 14 February 2011 10:16:12 Jim Meyering wrote:
>> >> I see what you mean, but invalid names seem important enough that I would
>> >> not want to be prompted -- not even with a warning -- about the patch
>> >> in question.
>> >
>> > On the other hand, immediately aborting when we see an invalid name (like
>> > in the current git) is also not appreciated?
>>
>> When it comes to security, even low-risk things like this,
>> I think it pays to be extra careful, even if that ends up
>> causing minor inconvenience.
>
> I guess I don't see the point. That's what the code in the current repo
> does, but apparently that's too much?
>
>> >> When being prompted, it is too easy to miss the preceding
>> >> warning among the already relatively verbose output.
>> >
>> > What harm does it do if the warning is overlooked?
>>
>> With a prompt, it's too easy for the naive user to type in some variant
>> of the invalid file name. Obviously neither you nor I would try
>> "../../f" when patch says that "../f" doesn't work, but for a beginner,
>> even ../../../etc/passwd might not raise an eyebrow. Issuing the prompt
>> makes abuse via social engineering a tiny bit easier.
>
> A person with that level of technical understanding might just as well apply a
> patch while in the root directory, no?
>
> Here's a patch that implements what I have in mind. Do you really think that
> this approach is too unsafe?
Perhaps not unsafe, but I now see that it can be improved...
Patch still stats the offending file.
For example, running this
printf '
--- /dev/null
+++ a/../z
@@ -0,0 +1 @@
+x
' | src/patch -f -p1 --dry-run
Usually prints the expected output:
Ignoring potentially dangerous file name ../z
can't find file to patch at input line 4
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
But if you create ../z, then it will print the new "Ignoring ..."
diagnostic twice. I.e., this
touch ../z
printf '
--- /dev/null
+++ a/../z
@@ -0,0 +1 @@
+x
' | src/patch -f -p1 --dry-run 2>&1|grep -c Ignoring
prints "2".
Thus, if you create a file named "z" in tests/
then "make check" will fail.
- [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Jim Meyering, 2011/02/10
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Andreas Gruenbacher, 2011/02/13
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Jim Meyering, 2011/02/14
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Andreas Gruenbacher, 2011/02/14
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Jim Meyering, 2011/02/14
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Andreas Gruenbacher, 2011/02/14
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Jim Meyering, 2011/02/14
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Andreas Gruenbacher, 2011/02/15
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Jim Meyering, 2011/02/16
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Andreas Gruenbacher, 2011/02/16
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line,
Jim Meyering <=
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Andreas Gruenbacher, 2011/02/16
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Jim Meyering, 2011/02/16
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Andreas Gruenbacher, 2011/02/16
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Jim Meyering, 2011/02/16
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Andreas Gruenbacher, 2011/02/16
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Jim Meyering, 2011/02/16
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Andreas Gruenbacher, 2011/02/17
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Jim Meyering, 2011/02/17
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Andreas Gruenbacher, 2011/02/17
- Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line, Jim Meyering, 2011/02/17