I'm going to accept these patches for more testing. If I understand correctly, the changes are that:
1. the check is performed in the outer layer (commands)
instead of within the inner layer (patches)
2. the check is now optional (and it is the possibility of disabling this check
that resolves issue177
The code for the check itself (is_malicious_path) has changed, but unless I've
got my boolean thinking wrong, it is exactly equivalent to the old one.
On Fri, Nov 10, 2006 at 23:00:19 +0100, [EMAIL PROTECTED] wrote:
> I was implementing an option to turn off the malicious file path
> check, for (if nothing else) backward compatibility with older
> versions (or more exactly, how they are sometimes used). Then I
> discovered that the check is not working properly, and that a
> proper way of doing the check is not so easy to implement.
> The main problem with the old way is the apply function in
> PatchApply is not the only "entry point" to applying patches.
It's not? That's interesting. What other entry points are there?
[ i'm not asking for a full list :-) ]
> But I remember something from when this problem was discussed the
> first time about doing it in the Slurpy.
Hmm... I have a vague intuition that the Slurpy would naturally protect
against writing outside the current tree (i.e. outside the working dir
or outside the pristine cache), but you would have to check how addslurp
works to be absolutely sure. I guess what it would not (naturally)
protect against is writing to _darcs whilst trying to write to the
working directory.
I guess two things one could do to pursue this train of thought is
1. Find the old thread where using the Slurpy was discussed
2. Learn more about functions like addslurp
Also, I'm not sure if protecting the Slurpy would be the right way to
go about it either. I say this a bit stupidly in that I see that
PatchApply.apply can apply to any WriteableDirectory, not just a
Slurpy. I guess this would mean IO or Slurpy for now.
> It unfortunately only checks
> remote patches fetched with Apply or Pull, but it does check
> them reliably.
Did you overlook Pull by any chance? I see only code for checking with
Check and Apply. Note that Pull does not invoke anything from Apply,
so either a refactor or a separate check would be necessary.
> +has_malicious_path :: Patch -> Bool
> +has_malicious_path patch =
> + let paths = map fn2fp (filenames patch) in
> + any is_malicious_path paths
This may be worth a point-free refactor.
> +is_malicious_path :: String -> Bool
> +is_malicious_path fp =
> + not $ is_explicitly_relative fp &&
> + null (intersect (breakup fp) [ "..", "_darcs" ])
I personally found the old code easier to understand. How about
something like this
is_malicious_path :: FilePath -> Bool
is_malicious_path fp =
not (is_explicitly_relative fp) ||
breakup fp `contains_any` [ "..", "_darcs" ]
where
contains_any a b = not . null $ intersect a b
--
Eric Kow http://www.loria.fr/~kow
PGP Key ID: 08AC04F9 Merci de corriger mon français.
pgpKvKoh8hP3m.pgp
Description: PGP signature
_______________________________________________ darcs-devel mailing list [email protected] http://www.abridgegame.org/cgi-bin/mailman/listinfo/darcs-devel
