grim/hgkeeper

Deny invalid path

12 months ago, aklitzing
5a19892df841
Parents 7485b844616f
Children 5f030be0a76c
Deny 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 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`

Reviewed at https://reviews.imfreedom.org/r/2422/
4 files changed, 23 insertions(+), 5 deletions(-)
  • +3 -0
    access/access.go
  • +14 -0
    access/repositories.go
  • +2 -2
    once/command.go
  • +4 -3
    ssh/commands/commands.go
    • --- a/access/access.go Wed Apr 12 23:57:06 2023 -0500
    +++ b/access/access.go Thu Apr 13 00:05:31 2023 -0500
    @@ -96,6 +96,9 @@
    func check(user, repo, action string) bool {
    // Normalize the repo to remove all trailing /'s and \'s.
    repo = strings.TrimRight(repo, "\\/")
    + if repo == "" {
    + return false
    + }
    r, err := enforcer.Enforce(user, repo, action)
    if err != nil {
    --- a/access/repositories.go Wed Apr 12 23:57:06 2023 -0500
    +++ b/access/repositories.go Thu Apr 13 00:05:31 2023 -0500
    @@ -5,8 +5,22 @@
    "path"
    "path/filepath"
    "strings"
    +
    + "go.uber.org/zap"
    )
    +func NormalizeRepo(reposPath, repoName string) (string, string) {
    + absPath := filepath.Join(reposPath, filepath.Clean(repoName))
    + realRepoName := strings.Trim(strings.TrimPrefix(absPath, reposPath), "\\/")
    +
    + if strings.HasPrefix(absPath, reposPath) && len(realRepoName) > 0 && !strings.Contains(repoName, "/../") {
    + return reposPath, realRepoName
    + }
    +
    + zap.S().Infof("repository invalid: %q", repoName)
    + return "", ""
    +}
    +
    func IsExistingRepo(repo string) bool {
    _, ok := repositories[repo]
    return ok
    --- a/once/command.go Wed Apr 12 23:57:06 2023 -0500
    +++ b/once/command.go Thu Apr 13 00:05:31 2023 -0500
    @@ -58,9 +58,9 @@
    switch cmd {
    case "hg serve":
    - return c.serve(g.ReposPath, args.Hg.Repo)
    + return c.serve(access.NormalizeRepo(g.ReposPath, args.Hg.Repo))
    case "hg init <repo>":
    - return c.init(g.ReposPath, args.Hg.Init.Repo)
    + return c.init(access.NormalizeRepo(g.ReposPath, args.Hg.Init.Repo))
    default:
    return fmt.Errorf("unsupported command %q", cmd)
    }
    --- a/ssh/commands/commands.go Wed Apr 12 23:57:06 2023 -0500
    +++ b/ssh/commands/commands.go Thu Apr 13 00:05:31 2023 -0500
    @@ -5,6 +5,7 @@
    "github.com/gliderlabs/ssh"
    + "keep.imfreedom.org/grim/hgkeeper/access"
    "keep.imfreedom.org/grim/hgkeeper/hg"
    )
    @@ -20,11 +21,11 @@
    switch pcmd {
    case "hg serve":
    - return NewServe(reposPath, values.Hg.Repo), nil
    + return NewServe(access.NormalizeRepo(reposPath, values.Hg.Repo)), nil
    case "hg init <repo>":
    - return NewInit(reposPath, values.Hg.Init.Repo), nil
    + return NewInit(access.NormalizeRepo(reposPath, values.Hg.Init.Repo)), nil
    case "rm <repo>":
    - return NewRemove(reposPath, values.Rm.Repo), nil
    + return NewRemove(access.NormalizeRepo(reposPath, values.Rm.Repo)), nil
    default:
    return nil, fmt.Errorf("unknown command %s", cmd)
    }