@cryptotaxi247 / netdata / commits / ac36b85fe

fix(go.d): resolve potential toctou vulnerability in binary path validation (#20798)

fix(go.d): resolve toctou vuln in binary path validation

Ilya Mashchenko committed Aug 11, 2025 at 10:44 UTC ac36b85fedd7624e02af3bc128f0dc135267125e
5 files changed +26 -22
src/go/plugin/go.d/collector/ap/init.go
+3 -2
@@ -35,11 +35,12 @@ func (c *Collector) initIwExec() (iwBinary, error) {
35 return nil, err
36 }
37
38 - if err := pathvalidate.ValidateBinaryPath(binPath); err != nil {
38 + validatedPath, err := pathvalidate.ValidateBinaryPath(binPath)
39 + if err != nil {
40 return nil, err
41 }
42
42 - iw := newIwExec(binPath, c.Timeout.Duration())
43 + iw := newIwExec(validatedPath, c.Timeout.Duration())
44
45 return iw, nil
46 }
src/go/plugin/go.d/collector/nvidia_smi/init.go
+3 -2
@@ -20,9 +20,10 @@ func (c *Collector) initNvidiaSmiExec() (nvidiaSmiBinary, error) {
20 binPath = path
21 }
22
23 - if err := pathvalidate.ValidateBinaryPath(binPath); err != nil {
23 + validatedPath, err := pathvalidate.ValidateBinaryPath(binPath)
24 + if err != nil {
25 return nil, err
26 }
27
27 - return newNvidiaSmiBinary(binPath, c.Config, c.Logger)
28 + return newNvidiaSmiBinary(validatedPath, c.Config, c.Logger)
29 }
src/go/plugin/go.d/collector/postfix/init.go
+3 -2
@@ -33,11 +33,12 @@ func (c *Collector) initPostqueueExec() (postqueueBinary, error) {
33 return nil, err
34 }
35
36 - if err := pathvalidate.ValidateBinaryPath(binPath); err != nil {
36 + validatedPath, err := pathvalidate.ValidateBinaryPath(binPath)
37 + if err != nil {
38 return nil, err
39 }
40
40 - pq := newPostqueueExec(binPath, c.Timeout.Duration())
41 + pq := newPostqueueExec(validatedPath, c.Timeout.Duration())
42 pq.Logger = c.Logger
43
44 return pq, nil
src/go/plugin/go.d/collector/zfspool/init.go
+3 -2
@@ -35,11 +35,12 @@ func (c *Collector) initZPoolCLIExec() (zpoolCli, error) {
35 return nil, err
36 }
37
38 - if err := pathvalidate.ValidateBinaryPath(binPath); err != nil {
38 + validatedPath, err := pathvalidate.ValidateBinaryPath(binPath)
39 + if err != nil {
40 return nil, err
41 }
42
42 - zpoolExec := newZpoolCLIExec(binPath, c.Timeout.Duration())
43 + zpoolExec := newZpoolCLIExec(validatedPath, c.Timeout.Duration())
44 zpoolExec.Logger = c.Logger
45
46 return zpoolExec, nil
src/go/plugin/go.d/pkg/pathvalidate/validate_unix.go
+14 -14
@@ -13,68 +13,68 @@ import (
13
14 // ValidateBinaryPath checks if a binary path is secure for execution.
15 // It verifies ownership, permissions, and directory security.
16 -func ValidateBinaryPath(path string) error {
16 +func ValidateBinaryPath(path string) (string, error) {
17 // Step 1: Resolve full symlink path
18 resolvedPath, err := filepath.EvalSymlinks(path)
19 if err != nil {
20 - return fmt.Errorf("failed to resolve symlink for %s: %w", path, err)
20 + return "", fmt.Errorf("failed to resolve symlink for %s: %w", path, err)
21 }
22
23 // Step 2: Resolve to absolute path
24 absPath, err := filepath.Abs(resolvedPath)
25 if err != nil {
26 - return fmt.Errorf("failed to resolve absolute path for %s: %w", resolvedPath, err)
26 + return "", fmt.Errorf("failed to resolve absolute path for %s: %w", resolvedPath, err)
27 }
28
29 // Step 3: Stat the resolved file
30 fileInfo, err := os.Stat(absPath)
31 if err != nil {
32 - return fmt.Errorf("binary stat error for %s: %w", absPath, err)
32 + return "", fmt.Errorf("binary stat error for %s: %w", absPath, err)
33 }
34
35 // Step 4: Check that it is a regular file
36 if !fileInfo.Mode().IsRegular() {
37 - return fmt.Errorf("binary at %s must be a regular file, not %s", absPath, fileInfo.Mode().String())
37 + return "", fmt.Errorf("binary at %s must be a regular file, not %s", absPath, fileInfo.Mode().String())
38 }
39
40 // Step 5: Check file ownership and permissions
41 fileStat, ok := fileInfo.Sys().(*syscall.Stat_t)
42 if !ok {
43 - return fmt.Errorf("unable to get file stat information for %s", absPath)
43 + return "", fmt.Errorf("unable to get file stat information for %s", absPath)
44 }
45 if fileStat.Uid != 0 {
46 - return fmt.Errorf("binary at %s must be owned by root (current uid: %d)", absPath, fileStat.Uid)
46 + return "", fmt.Errorf("binary at %s must be owned by root (current uid: %d)", absPath, fileStat.Uid)
47 }
48
49 if perm := fileInfo.Mode().Perm(); perm&0022 != 0 {
50 - return fmt.Errorf("binary at %s must not be writable by group/others (current permissions: %s / %04o)",
50 + return "", fmt.Errorf("binary at %s must not be writable by group/others (current permissions: %s / %04o)",
51 absPath, fileInfo.Mode().String(), perm)
52 }
53
54 // Step 6: Check executable bit
55 if fileInfo.Mode().Perm()&0111 == 0 {
56 - return fmt.Errorf("binary at %s must be executable", absPath)
56 + return "", fmt.Errorf("binary at %s must be executable", absPath)
57 }
58
59 // Step 7: Check parent directory
60 dir := filepath.Dir(absPath)
61 dirInfo, err := os.Stat(dir)
62 if err != nil {
63 - return fmt.Errorf("directory stat error for %s: %w", dir, err)
63 + return "", fmt.Errorf("directory stat error for %s: %w", dir, err)
64 }
65
66 dirStat, ok := dirInfo.Sys().(*syscall.Stat_t)
67 if !ok {
68 - return fmt.Errorf("unable to get directory stat information for %s", dir)
68 + return "", fmt.Errorf("unable to get directory stat information for %s", dir)
69 }
70 if dirStat.Uid != 0 {
71 - return fmt.Errorf("directory %s must be owned by root (current uid: %d)", dir, dirStat.Uid)
71 + return "", fmt.Errorf("directory %s must be owned by root (current uid: %d)", dir, dirStat.Uid)
72 }
73
74 if perm := dirInfo.Mode().Perm(); perm&0022 != 0 {
75 - return fmt.Errorf("directory %s must not be writable by group/others (current permissions: %s / %04o)",
75 + return "", fmt.Errorf("directory %s must not be writable by group/others (current permissions: %s / %04o)",
76 dir, dirInfo.Mode().String(), perm)
77 }
78
79 - return nil
79 + return absPath, nil
80 }