mirror of
https://github.com/moby/moby.git
synced 2022-11-09 12:21:53 -05:00
7114360901
This fixes an inifinite loop in mkdirAs(), used by `MkdirAllAndChown`,
`MkdirAndChown`, and `MkdirAllAndChownNew`, as well as directories being
chown'd multiple times when relative paths are used.
The for loop in this function was incorrectly assuming that;
1. `filepath.Dir()` would always return the parent directory of any given path
2. traversing any given path to ultimately result in "/"
While this is correct for absolute and "cleaned" paths, both assumptions are
incorrect in some variations of "path";
1. for paths with a trailing path-separator ("some/path/"), or dot ("."),
`filepath.Dir()` considers the (implicit) "." to be a location _within_ the
directory, and returns "some/path" as ("parent") directory. This resulted
in the path itself to be included _twice_ in the list of paths to chown.
2. for relative paths ("./some-path", "../some-path"), "traversing" the path
would never end in "/", causing the for loop to run indefinitely:
```go
// walk back to "/" looking for directories which do not exist
// and add them to the paths array for chown after creation
dirPath := path
for {
dirPath = filepath.Dir(dirPath)
if dirPath == "/" {
break
}
if _, err := os.Stat(dirPath); err != nil && os.IsNotExist(err) {
paths = append(paths, dirPath)
}
}
```
A _partial_ mitigation for this would be to use `filepath.Clean()` before using
the path (while `filepath.Dir()` _does_ call `filepath.Clean()`, it only does so
_after_ some processing, so only cleans the result). Doing so would prevent the
double chown from happening, but would not prevent the "final" path to be "."
or ".." (in the relative path case), still causing an infinite loop, or
additional checks for "." / ".." to be needed.
| path | filepath.Dir(path) | filepath.Dir(filepath.Clean(path)) |
|----------------|--------------------|------------------------------------|
| some-path | . | . |
| ./some-path | . | . |
| ../some-path | .. | .. |
| some/path/ | some/path | some |
| ./some/path/ | some/path | some |
| ../some/path/ | ../some/path | ../some |
| some/path/. | some/path | some |
| ./some/path/. | some/path | some |
| ../some/path/. | ../some/path | ../some |
| /some/path/ | /some/path | /some |
| /some/path/. | /some/path | /some |
Instead, this patch adds a `filepath.Abs()` to the function, so make sure that
paths are both cleaned, and not resulting in an infinite loop.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit
|
||
|---|---|---|
| .. | ||
| idtools.go | ||
| idtools_test.go | ||
| idtools_unix.go | ||
| idtools_unix_test.go | ||
| idtools_windows.go | ||
| usergroupadd_linux.go | ||
| usergroupadd_unsupported.go | ||
| utils_unix.go | ||