Skip to content

Commit 3d4b240

Browse files
jarpsimoesjarpsimoes
authored
Merge pull request #10 from jarpsimoes/feature/8-fix-code-scanning-alert-uncontrolled-data-used-in-path-expression
Feature/8 fix code scanning alert uncontrolled data used in path expression
2 parents 78b7511 + 47df198 commit 3d4b240

File tree

6 files changed

+177
-24
lines changed

6 files changed

+177
-24
lines changed

‎README.md‎

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,15 @@ selected branch (in variable REPO_BRANCH) and can be pulled new version on defin
1111

1212
## RELEASE NOTES: v0.0.3-alpha
1313

14-
| Feature | Description |
15-
|---------|---------------------------------------------------|
16-
| Done | Add support to proxy redirect with HTTPS backends |
17-
| FIX | Bump golang.org/x/text from 0.3.7 to 0.3.8 |
18-
14+
| Feature | Description |
15+
|---------|--------------------------------------------------------------------------------------------------------------------|
16+
| Done | Add support to proxy redirect with HTTPS backends |
17+
| FIXED | Bump golang.org/x/text from 0.3.7 to 0.3.8 |
18+
| FIXED | Bump github.com/stretchr/testify from 1.7.0 to 1.7.1 |
19+
| FIXED | Bump golang.org/x/sys from 0.0.0-20220209214540-3681064d5158 to 0.1.0 |
20+
| FIXED | Bump golang.org/x/crypto from 0.0.0-20220214200702-86341886e292 to 0.1.0 |
21+
| FIXED | Bump golang.org/x/net from 0.0.0-20220127200216-cd36cc0744dd to 0.17.0 in |
22+
| FIXED | Vulnerability issue Uncontrolled data used in path [Issue](https://github.com/jarpsimoes/git-http-server/issues/8) |
1923

2024

2125
## Authentication Methods
@@ -25,22 +29,22 @@ The GIT-HttpServer only support basic authentication on repositories by protocol
2529
## Configuration
2630

2731
### Environment Variables
28-
| Name | Description | Default | Mandatory |
29-
|---------------------------|----------------------------------------------------------|---------------------------------------------------|-----------|
30-
| PATH_CLONE | Set clone path | _clone | Yes |
31-
| PATH_PULL | Set pull path | _pull | Yes |
32-
| PATH_VERSION | Set get git commit version path | _version | Yes |
33-
| PATH_WEBHOOK | Set webhook path | _hook | Yes |
34-
| PATH_HEALTH | Set health check path | _health | Yes |
35-
| REPO_BRANCH | Set default branch to clone content | main | No |
36-
| REPO_TARGET_FOLDER | Set folder to clone source | target-git | No |
37-
| REPO_URL | Set url as a source origin | https://github.com/jarpsimoes/git-http-server.git | No |
38-
| REPO_USERNAME | Set username or token identifier to basic authentication | N/D | No |
39-
| REPO_PASSWORD | Set password or token to basic authentication | N/D | No |
40-
| HTTP_PORT | Set port to expose content | 8081 | Yes |
41-
| GHS_CUSTOM_PATH_<path> | Custom path to work as a proxy server | N/D | No |
42-
| GHS_CUSTOM_REWRITE_<path> | Set to remove from proxy request base path | N/D | No |
43-
| FOLDER_ROOT | Select root folder inside cloned repository | $REPO_TARGET_FOLDER/ | No |
32+
| Name | Description | Default | Mandatory |
33+
|---------------------------|------------------------------------------------------------------------|---------------------------------------------------|-----------|
34+
| PATH_CLONE | Set clone path | _clone | Yes |
35+
| PATH_PULL | Set pull path | _pull | Yes |
36+
| PATH_VERSION | Set get git commit version path | _version | Yes |
37+
| PATH_WEBHOOK | Set webhook path | _hook | Yes |
38+
| PATH_HEALTH | Set health check path | _health | Yes |
39+
| REPO_BRANCH | Set default branch to clone content | main | No |
40+
| REPO_TARGET_FOLDER | Set folder to clone source (only allowed letters, numbers, underscore) | target-git | No |
41+
| REPO_URL | Set url as a source origin | https://github.com/jarpsimoes/git-http-server.git | No |
42+
| REPO_USERNAME | Set username or token identifier to basic authentication | N/D | No |
43+
| REPO_PASSWORD | Set password or token to basic authentication | N/D | No |
44+
| HTTP_PORT | Set port to expose content | 8081 | Yes |
45+
| GHS_CUSTOM_PATH_<path> | Custom path to work as a proxy server | N/D | No |
46+
| GHS_CUSTOM_REWRITE_<path> | Set to remove from proxy request base path | N/D | No |
47+
| FOLDER_ROOT **(Removed)** | The base path will be always root folder of the application | REMOVED | N/D |
4448

4549

4650
## Implementation

‎src/utils/custom_path_test.go‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ import (
1111
func TestGetAllCustomPaths(t *testing.T) {
1212
os.Setenv("GHS_CUSTOM_PATH_images/digilex-infordoc-images", "https://storage.gra.cloud.ovh.net/v1/AUTH_ed33ec9e34c64b54aca49d6fcb6dc4c8/infordoc-img")
1313
os.Setenv("GHS_CUSTOM_REWRITE_images/digilex-infordoc-images", "")
14-
os.Setenv("GHS_CUSTOM_PATH_images.digilex-infordoc-images-1", "https://storage.gra.cloud.ovh.net/v1/AUTH_ed33ec9e34c64b54aca49d6fcb6dc4c8/img")
15-
os.Setenv("GHS_CUSTOM_PATH_/images/digilex-infordoc-images-2", "https://storage.gra.cloud.ovh.net/v1/AUTH_ed33ec9e34c64b54aca49d6fcb6dc4c8/infordoc")
14+
os.Setenv("GHS_CUSTOM_PATH_images.digilex-infordoc/1", "https://storage.gra.cloud.ovh.net/v1/AUTH_ed33ec9e34c64b54aca49d6fcb6dc4c8/img")
15+
os.Setenv("GHS_CUSTOM_PATH_/images/digilex-infordoc-img", "https://storage.gra.cloud.ovh.net/v1/AUTH_ed33ec9e34c64b54aca49d6fcb6dc4c8/infordoc")
16+
1617
result := GetCustomPathsInstance()
1718

1819
assert.True(t, len(*result) == 3)
@@ -25,3 +26,32 @@ func TestGetAllCustomPaths(t *testing.T) {
2526
}
2627

2728
}
29+
func TestFindPath(t *testing.T) {
30+
os.Setenv("GHS_CUSTOM_PATH_images/digilex-infordoc-images", "https://storage.gra.cloud.ovh.net/v1/AUTH_ed33ec9e34c64b54aca49d6fcb6dc4c8/infordoc-img")
31+
os.Setenv("GHS_CUSTOM_REWRITE_images/digilex-infordoc-images", "")
32+
os.Setenv("GHS_CUSTOM_PATH_images.digilex-infordoc/1", "https://storage.gra.cloud.ovh.net/v1/AUTH_ed33ec9e34c64b54aca49d6fcb6dc4c8/img")
33+
os.Setenv("GHS_CUSTOM_PATH_/images/digilex-infordoc-img", "https://storage.gra.cloud.ovh.net/v1/AUTH_ed33ec9e34c64b54aca49d6fcb6dc4c8/infordoc")
34+
35+
result, path := FindPath("/images/digilex-infordoc-images")
36+
37+
assert.True(t, result)
38+
assert.True(t, path.GetTarget() == "https://storage.gra.cloud.ovh.net/v1/AUTH_ed33ec9e34c64b54aca49d6fcb6dc4c8/infordoc-img")
39+
assert.True(t, path.IsRewrite())
40+
41+
result1, path1 := FindPath("/images/digilex-infordoc/1")
42+
43+
assert.True(t, result1)
44+
assert.True(t, path1.GetTarget() == "https://storage.gra.cloud.ovh.net/v1/AUTH_ed33ec9e34c64b54aca49d6fcb6dc4c8/img")
45+
assert.True(t, path1.IsRewrite() == false)
46+
47+
result2, path2 := FindPath("/images/digilex-infordoc-img")
48+
49+
assert.True(t, result2)
50+
assert.True(t, path2.GetTarget() == "https://storage.gra.cloud.ovh.net/v1/AUTH_ed33ec9e34c64b54aca49d6fcb6dc4c8/infordoc")
51+
assert.True(t, path2.IsRewrite() == false)
52+
53+
result3, _ := FindPath("/images/test/digilex-infordoc-img")
54+
55+
assert.False(t, result3)
56+
57+
}

‎src/utils/environment_config.go‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"github.com/go-git/go-git/v5/plumbing/transport/http"
77
"log"
88
"os"
9+
"path/filepath"
10+
"runtime"
911
"sync"
1012
"time"
1113
)
@@ -51,6 +53,11 @@ var baseRouteConfigInstance *BaseRouteConfig
5153
var baseRepositoryConfigInstance *BaseRepositoryConfig
5254
var basicAuthenticationMethod *BasicAuthenticationMethod
5355
var healthCheckControl *HealthCheckControl
56+
var pathSecurityCheck *PathSecurityCheck
57+
var (
58+
_, b, _, _ = runtime.Caller(0)
59+
basePath = filepath.Dir(b)
60+
)
5461

5562
// UpdateState [HealthCheckControl] it's a function to update Status
5663
func (hcc *HealthCheckControl) UpdateState(status bool) {
@@ -183,9 +190,16 @@ func GetRepositoryConfigInstance() *BaseRepositoryConfig {
183190
repoURL: os.Getenv("REPO_URL"),
184191
branch: os.Getenv("REPO_BRANCH"),
185192
targetFolder: os.Getenv("REPO_TARGET_FOLDER"),
186-
rootFolder: os.Getenv("FOLDER_ROOT"),
193+
rootFolder: basePath,
187194
}
188195

196+
if pathSecurityCheck.IsValidPath(baseRepositoryConfigInstance.targetFolder) {
197+
log.Printf("[BaseRepositoryConfigInstance] Target folder %v is valid", baseRepositoryConfigInstance.targetFolder)
198+
} else {
199+
log.Printf("[BaseRepositoryConfigInstance] Target folder %v is invalid", baseRepositoryConfigInstance.targetFolder)
200+
log.Printf("[BaseRepositoryConfigInstance] Will be changed to target")
201+
baseRepositoryConfigInstance.targetFolder = "target_folder"
202+
}
189203
}
190204
}
191205

‎src/utils/environment_config_test.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,17 @@ func TestGetRepositoryConfigInstance(t *testing.T) {
4343
assert.Equal(t, "https://test.com/repo.git", repositoryConfigInstance1.GetRepo(), "Check repo url")
4444
assert.Equal(t, "main", repositoryConfigInstance1.GetBranch(), "Check repo url")
4545
assert.Equal(t, "test1", repositoryConfigInstance1.GetTargetFolder(), "Check repo url")
46+
47+
}
48+
func TestGetRepositoryConfigInstanceInvalid(t *testing.T) {
49+
baseRepositoryConfigInstance = nil
50+
51+
os.Setenv("REPO_URL", "https://test.com/repo.git")
52+
os.Setenv("REPO_BRANCH", "main")
53+
os.Setenv("REPO_TARGET_FOLDER", "test1/test2")
54+
55+
repositoryConfigInstance2 := GetRepositoryConfigInstance()
56+
assert.Equal(t, "target_folder", repositoryConfigInstance2.GetTargetFolder(), "Check repo url")
4657
}
4758
func TestGetBasicAuthenticationMethodInstance(t *testing.T) {
4859
os.Setenv("REPO_USERNAME", os.Getenv("ACCESS_USERNAME"))

‎src/utils/path_security_check.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package utils
2+
3+
import (
4+
"regexp"
5+
)
6+
7+
type PathSecurityCheck struct {
8+
}
9+
10+
// IsValidPath checks if the input string adheres to the specified rules.
11+
func (psc *PathSecurityCheck) IsValidPath(input string) bool {
12+
// Rule 1: Do not allow more than a single "." character.
13+
if countDots := psc.countOccurrences(input, '.'); countDots > 1 {
14+
return false
15+
}
16+
17+
if psc.containsDirectorySeparator(input) {
18+
return false
19+
}
20+
21+
allowList := []*regexp.Regexp{
22+
regexp.MustCompile(`^[a-zA-Z0-9_-]+$`), // Alphanumeric characters, underscore, and hyphen.
23+
regexp.MustCompile(`^([a-zA-Z0-9_-]+\.){0,2}[a-zA-Z0-9_-]+$`), // Allow up to two dots in the filename.
24+
}
25+
26+
for _, pattern := range allowList {
27+
if pattern.MatchString(input) {
28+
return true
29+
}
30+
}
31+
32+
return false
33+
}
34+
35+
// countOccurrences counts the occurrences of a specific character in a string.
36+
func (psc *PathSecurityCheck) countOccurrences(s string, c byte) int {
37+
count := 0
38+
for i := 0; i < len(s); i++ {
39+
if s[i] == c {
40+
count++
41+
}
42+
}
43+
return count
44+
}
45+
46+
// containsDirectorySeparator checks if the input string contains directory separators.
47+
func (psc *PathSecurityCheck) containsDirectorySeparator(s string) bool {
48+
return psc.containsAny(s, []byte{'/', '\\'})
49+
}
50+
51+
// containsAny checks if the input string contains any of the specified characters.
52+
func (psc *PathSecurityCheck) containsAny(s string, chars []byte) bool {
53+
for i := 0; i < len(s); i++ {
54+
for _, c := range chars {
55+
if s[i] == c {
56+
return true
57+
}
58+
}
59+
}
60+
return false
61+
}

‎src/utils/path_security_test.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package utils
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func TestIsValidPath(t *testing.T) {
8+
tests := []struct {
9+
input string
10+
expected bool
11+
}{
12+
{"valid_path123", true},
13+
{"invalid_path/with_slash", false},
14+
{"invalid_path/with/slash", false},
15+
{"invalid/path/with/slash", false},
16+
{"invalid.path.with.multiple.dots", false},
17+
{"another.invalid.path.with.multiple.dots", false},
18+
{"valid_path_with_underscore", true},
19+
{"valid-path_with-hyphen", true},
20+
{"valid.path_with_underscore-hyphen", true},
21+
}
22+
23+
pathSecurityCheck := PathSecurityCheck{}
24+
25+
for _, test := range tests {
26+
t.Run(test.input, func(t *testing.T) {
27+
result := pathSecurityCheck.IsValidPath(test.input)
28+
if result != test.expected {
29+
t.Errorf("Expected IsValidPath(%s) to be %v, but got %v", test.input, test.expected, result)
30+
}
31+
})
32+
}
33+
}

0 commit comments

Comments
 (0)