Skip to content

Commit 95d43ff

Browse files
Removed symlinks and fd, switching to plain getfacl
1 parent c2a8ab6 commit 95d43ff

File tree

1 file changed

+21
-58
lines changed

1 file changed

+21
-58
lines changed

internal/traversal/traversal.go

Lines changed: 21 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,10 @@ import (
66
"os/exec"
77
"path/filepath"
88
"strings"
9-
// "syscall"
109

1110
"go.uber.org/zap"
1211

1312
"github.com/PythonHacker24/linux-acl-management-backend/config"
14-
1513
)
1614

1715
/* comprehensive list of dangerous characters */
@@ -29,30 +27,20 @@ func ListFiles(path string, userID string) ([]FileEntry, error) {
2927
/* clean the path to prevent directory traversal */
3028
fullPath = filepath.Clean(fullPath)
3129

32-
/* evaluate symlinks to get the real path */
33-
realPath, err := filepath.EvalSymlinks(fullPath)
34-
if err != nil {
35-
zap.L().Warn("Failed to evaluate symlinks",
36-
zap.String("path", fullPath),
37-
zap.Error(err),
38-
)
39-
return nil, fmt.Errorf("invalid path or broken symlink: %w", err)
40-
}
41-
42-
/* ensure the resulting path is still within the basePath (prevent directory traversal) */
43-
if !strings.HasPrefix(realPath, filepath.Clean(config.BackendConfig.AppInfo.BasePath)) {
30+
/* ensure the path is still within the basePath (prevent directory traversal) */
31+
if !strings.HasPrefix(fullPath, filepath.Clean(config.BackendConfig.AppInfo.BasePath)) {
4432
zap.L().Warn("Path traversal attempt detected",
4533
zap.String("path", path),
46-
zap.String("resolved_path", realPath),
34+
zap.String("full_path", fullPath),
4735
)
4836
return nil, fmt.Errorf("access denied: path outside allowed directory")
4937
}
5038

5139
/* list all the files in the given directory */
52-
files, err := os.ReadDir(realPath)
40+
files, err := os.ReadDir(fullPath)
5341
if err != nil {
5442
zap.L().Error("Failed to read directory",
55-
zap.String("path", realPath),
43+
zap.String("path", fullPath),
5644
zap.Error(err),
5745
)
5846
return nil, fmt.Errorf("failed to read directory: %w", err)
@@ -61,47 +49,23 @@ func ListFiles(path string, userID string) ([]FileEntry, error) {
6149
/* retrieve information for each file in the directory */
6250
for _, f := range files {
6351
entryPath := filepath.Join(path, f.Name())
64-
fullEntryPath := filepath.Join(realPath, f.Name())
65-
66-
/* evaluate symlinks for each entry */
67-
realEntryPath, err := filepath.EvalSymlinks(fullEntryPath)
68-
if err != nil {
69-
zap.L().Warn("Failed to evaluate symlinks for entry",
70-
zap.String("entry", f.Name()),
71-
zap.Error(err),
72-
)
73-
continue
74-
}
52+
fullEntryPath := filepath.Join(fullPath, f.Name())
7553

7654
/* verify the entry is still within allowed directory */
77-
if !strings.HasPrefix(realEntryPath, filepath.Clean(config.BackendConfig.AppInfo.BasePath)) {
78-
zap.L().Warn("Entry symlink points outside allowed directory",
55+
if !strings.HasPrefix(fullEntryPath, filepath.Clean(config.BackendConfig.AppInfo.BasePath)) {
56+
zap.L().Warn("Entry path outside allowed directory",
7957
zap.String("entry", f.Name()),
80-
zap.String("resolved_path", realEntryPath),
58+
zap.String("full_path", fullEntryPath),
8159
)
8260
continue
8361
}
8462

85-
/* Open the file with O_NOFOLLOW to prevent symlink races */
86-
// file, err := os.OpenFile(realEntryPath, os.O_RDONLY|syscall.O_NOFOLLOW, 0)
87-
file, err := os.Open(realEntryPath)
88-
if err != nil {
89-
zap.L().Warn("Failed to open file",
90-
zap.String("path", realEntryPath),
91-
zap.Error(err),
92-
)
93-
continue
94-
}
95-
defer file.Close()
96-
97-
/* Get file descriptor for further operations */
98-
fd := file.Fd()
99-
100-
/* check ACL access using the file descriptor */
101-
isOwner, err := isOwnerFd(fd, realEntryPath, userID)
63+
/* check ACL access using the file path */
64+
isOwner, err := isOwner(fullEntryPath, userID)
10265
if err != nil {
103-
zap.L().Error("Failed to check ownership",
104-
zap.String("path", realEntryPath),
66+
zap.L().Warn("Failed to check ownership, skipping file",
67+
zap.String("path", fullEntryPath),
68+
zap.String("user", userID),
10569
zap.Error(err),
10670
)
10771
continue
@@ -111,11 +75,11 @@ func ListFiles(path string, userID string) ([]FileEntry, error) {
11175
continue
11276
}
11377

114-
/* get file information using the same file descriptor */
115-
info, err := file.Stat()
78+
/* get file information */
79+
info, err := os.Stat(fullEntryPath)
11680
if err != nil {
11781
zap.L().Warn("Error while getting file information",
118-
zap.String("path", realEntryPath),
82+
zap.String("path", fullEntryPath),
11983
zap.Error(err),
12084
)
12185
continue
@@ -134,10 +98,9 @@ func ListFiles(path string, userID string) ([]FileEntry, error) {
13498
}
13599

136100
/*
137-
checks if the user is the owner of the file using a file descriptor
138-
this reduces race conditions by operating on an already-open file
101+
checks if the user is the owner of the file using getfacl
139102
*/
140-
func isOwnerFd(fd uintptr, filePath string, userCN string) (bool, error) {
103+
func isOwner(filePath string, userCN string) (bool, error) {
141104
cleanPath := filepath.Clean(filePath)
142105

143106
/* validation to ensure that the path doesn't contain dangerous characters */
@@ -151,8 +114,8 @@ func isOwnerFd(fd uintptr, filePath string, userCN string) (bool, error) {
151114
}
152115
}
153116

154-
/* get the file's ACL using getfacl with the file descriptor */
155-
cmd := exec.Command("getfacl", fmt.Sprintf("/proc/self/fd/%d", fd))
117+
/* get the file's ACL using getfacl with the file path directly */
118+
cmd := exec.Command("getfacl", cleanPath)
156119
output, err := cmd.Output()
157120
if err != nil {
158121
zap.L().Error("Failed to execute getfacl",

0 commit comments

Comments
 (0)