grim/hgkeeper

Normalize paths before passing them to the authorization checker

15 months ago, Gary Kramlich
e33f7739ab49
Parents c273bc3f7bd1
Children 22d5b3872b43
Normalize paths before passing them to the authorization checker

This bug allowed attackers to bypass deny rules by adding a trailing / to the
repository which depending on the policy could grant them access to said
repository.
2 files changed, 9 insertions(+), 1 deletions(-)
  • +5 -1
    ChangeLog
  • +4 -0
    access/access.go
    • --- a/ChangeLog Wed Jan 04 05:46:21 2023 -0600
    +++ b/ChangeLog Wed Jan 04 07:21:26 2023 -0600
    @@ -1,8 +1,12 @@
    1.0.1: (????-??-??)
    - * Add a warning message when a duplicate key is loaded. Not behavior changes
    + * Add a warning message when a duplicate key is loaded. No behavior changes
    were made so that users depending on the broken behavior can continue
    working. (HGKEEPER-22)
    * Removed the mercurial 6.1 pin from the container image.
    + * Normalize paths before passing them to the authorization checker. This bug
    + allowed attackers to read repositories they shouldn't have been able to by
    + adding a / on to the end of the path. The hgkeeper repo in the default
    + configuration is susceptible to this attack.
    1.0.0: (2022-06-21)
    * Official first release!
    --- a/access/access.go Wed Jan 04 05:46:21 2023 -0600
    +++ b/access/access.go Wed Jan 04 07:21:26 2023 -0600
    @@ -4,6 +4,7 @@
    "io/ioutil"
    "os"
    "path/filepath"
    + "strings"
    "sync"
    log "github.com/sirupsen/logrus"
    @@ -87,6 +88,9 @@
    }
    func check(user, repo, action string) bool {
    + // Normalize the repo to remove all trailing /'s and \'s.
    + repo = strings.TrimRight(repo, "\\/")
    +
    r, err := enforcer.Enforce(user, repo, action)
    if err != nil {
    log.Errorf(