Summary

Deny invalid path

Review Request #2422 — Created April 4, 2023 and submitted April 13, 2023, 5:06 a.m.

Information

Owner

aklitzing@gmail.com

Repository

grim/hgkeeper

Branch

default

Bugs

Depends On

~~2421~~

Blocks

2424

Reviewers

Groups

People

grim

Description

If an authenticated user calls hg init hg.host.com/dummy/../../../etc
it will create the repository in another root directory if the process of
hgkeeper has permissions for this.
This could be an attack to the server.

Also hgkeeper admin repository can be overriden like this.
hg init ssh://hg.host.com/dummy/../hgkeeper/keys

Testing Done

Commits

Summary	ID	Author
Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	2003064d3ef00d9fa0eb452880164e1c8e26facb	Andre Klitzing

Issues

Description	From	Last Updated
mh, that is a bad fix... let's retry...	aklitzing@gmail.com	April 4, 2023, 10:01 p.m.
we probably want to use filepath.Clean here on repoName at a minimum.	grim	April 10, 2023, 10:42 p.m.
not really a fan of a security related function being duplicated. I know this is because there's no good place …	grim	April 12, 2023, 10:49 p.m.

Commits:

	Summary	ID	Author
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	e60f11824ab0a686ea7657cc9e61dbdf0ae511c2	Andre Klitzing
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	dffeb56c7a24f40d322717071229c171a3f52574	Andre Klitzing

Diff:

Revision 2 (+10)

Show changes

ssh/commands/init.go

Description:

~		If an authenticated user calls "hg init hg.host.com/dummy/../../../etc"
	~	If an authenticated user calls `hg init hg.host.com/dummy/../../../etc`
		it will create the repository in another root directory if the process of
		hgkeeper has permissions for this.
		This could be an attack to the server.
	+
	+	Also hgkeeper admin repository can be overriden like this.
	+	`hg init ssh://hg.host.com/dummy/../hgkeeper/keys`

In /r/2421/diff/1/#index_header I check if the repository is known.
That should be checked here, too. It will always be a problem if someone creates a new repository in a sub-directory of another repository.

Testing Done:

Container tested only... maybe "once/command.go" need to be adjusted, too.

Show all issues
```
mh, that is a bad fix... let's retry...
```

Summary:

-	Deny "hg init" for invalid path
+	Deny invalid path

Commits:

	Summary	ID	Author
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	dffeb56c7a24f40d322717071229c171a3f52574	Andre Klitzing
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	c5f114c462eda7aeb347a68541c8d53c28e6a168	Andre Klitzing

Diff:

Revision 3 (+38 -8)

Show changes

	ssh/commands/commands.go
	ssh/commands/init.go
	ssh/commands/serve.go

Commits:

	Summary	ID	Author
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	c5f114c462eda7aeb347a68541c8d53c28e6a168	Andre Klitzing
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	6e762c86354f760c1707fff79dd802aacdd0a815	Andre Klitzing

Diff:

Revision 4 (+38 -16)

Show changes

	access/access.go
	ssh/commands/commands.go
	ssh/commands/init.go
	ssh/commands/serve.go

Commits:

	Summary	ID	Author
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	6e762c86354f760c1707fff79dd802aacdd0a815	Andre Klitzing
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	2d7cd54a56663e707bd230b08460c4f325d87779	Andre Klitzing

Diff:

Revision 5 (+38 -8)

Show changes

	ssh/commands/commands.go
	ssh/commands/init.go
	ssh/commands/serve.go

ssh/commands/commands.go (Diff revision 5)

Show all issues

we probably want to use filepath.Clean here on repoName at a minimum.

Commits:

	Summary	ID	Author
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	2d7cd54a56663e707bd230b08460c4f325d87779	Andre Klitzing
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	4027c61d978cbcc913536121dc5764d8ead24476	Andre Klitzing

Diff:

Revision 6 (+40 -8)

Show changes

	ssh/commands/commands.go
	ssh/commands/init.go
	ssh/commands/serve.go

Commits:

	Summary	ID	Author
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	4027c61d978cbcc913536121dc5764d8ead24476	Andre Klitzing
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	59a61ba161ddcd6d2de505eb1458a027bb509fdd	Andre Klitzing

Diff:

Revision 7 (+38 -8)

Show changes

	ssh/commands/commands.go
	ssh/commands/init.go
	ssh/commands/serve.go

Commits:

	Summary	ID	Author
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	59a61ba161ddcd6d2de505eb1458a027bb509fdd	Andre Klitzing
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	135c74678a57f46cdfcdf8b95f77eb304172cea8	Andre Klitzing

Diff:

Revision 8 (+70 -12)

Show changes

	once/command.go
	ssh/commands/commands.go
	ssh/commands/init.go
	ssh/commands/serve.go

Testing Done:

Container tested only... maybe "once/command.go" need to be adjusted, too.

Commits:

	Summary	ID	Author
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	135c74678a57f46cdfcdf8b95f77eb304172cea8	Andre Klitzing
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	01c4b79434d4b4aeabbb46a024251b6a7bdd1f35	Andre Klitzing

Diff:

Revision 9 (+70 -12)

Show changes

	once/command.go
	ssh/commands/commands.go
	ssh/commands/init.go
	ssh/commands/serve.go

ssh/commands/commands.go (Diff revision 9)

Show all issues

not really a fan of a security related function being duplicated. I know this is because there's no good place to put it right now. Maybe a normalize package?

aklitzing@gmail.com April 12, 2023, 10:49 p.m.

Yeah, I didn't like it, too. I moved it to new repositories.go as a normal func instead of another lambda call. So it uses the normal "canInit/canRead/canRemove" for permission which will fail for empty/invalid path.

Commits:

	Summary	ID	Author
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	01c4b79434d4b4aeabbb46a024251b6a7bdd1f35	Andre Klitzing
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	040d7ca6efa5a5dcb90c0d557cbd808aad2e91de	Andre Klitzing

Depends On:

+	~~2421 - Introduce "rm <repo>" command~~

Diff:

Revision 10 (+40 -10)

Show changes

	access/repositories.go
	once/command.go
	ssh/commands/commands.go

Commits:

	Summary	ID	Author
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	040d7ca6efa5a5dcb90c0d557cbd808aad2e91de	Andre Klitzing
	Deny "hg init" for invalid path If an authenticated user calls "hg init hg.host.com/dummy/../../../etc" it will create the repository in another root directory. If the process of hgkeeper as permissions it could lead to overwrite system files.	2003064d3ef00d9fa0eb452880164e1c8e26facb	Andre Klitzing

Diff:

Revision 11 (+46 -10)

Show changes

	access/access.go
	access/repositories.go
	once/command.go
	ssh/commands/commands.go

Ship it!

```
Ship It!
```
```
Nice work, thanks!!
```

Review Board 6.0.2

Deny invalid path

Information

Reviewers

Commits:

Diff:

Description:

Testing Done:

Summary:

Commits:

Diff:

Commits:

Diff:

Commits:

Diff:

Commits:

Diff:

Commits:

Diff:

Commits:

Diff:

Testing Done:

Commits:

Diff:

Commits:

Depends On:

Diff:

Commits:

Diff:

Status: Closed (submitted)