Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
eb0ff3aa69 | ||
|
|
c76e7dcd2e |
@@ -156,12 +156,33 @@ func runValidateDocmap(args []string) int {
|
|||||||
// window between the Lstat size check in validateDocmapPath and the file open
|
// window between the Lstat size check in validateDocmapPath and the file open
|
||||||
// here. The limit is maxDocmapBytes+1 so we can detect a file that grew past
|
// here. The limit is maxDocmapBytes+1 so we can detect a file that grew past
|
||||||
// the cap after the stat without reading unbounded bytes.
|
// the cap after the stat without reading unbounded bytes.
|
||||||
|
//
|
||||||
|
// Defense-in-depth: stat the path immediately before and after open so we can
|
||||||
|
// detect a file swap between validateDocmapPath's validation and this open via
|
||||||
|
// os.SameFile. An attacker with workspace write access could otherwise replace
|
||||||
|
// the validated file with a symlink in the gap between validation and use.
|
||||||
|
preStat, err := os.Lstat(resolvedDocmap)
|
||||||
|
if err != nil {
|
||||||
|
fmt.Fprintf(errWriter, "Error: failed to stat docmap before open %q: %v\n", *docmapFlag, err)
|
||||||
|
return 2
|
||||||
|
}
|
||||||
f, err := os.Open(resolvedDocmap)
|
f, err := os.Open(resolvedDocmap)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
fmt.Fprintf(errWriter, "Error: failed to open docmap %q: %v\n", *docmapFlag, err)
|
fmt.Fprintf(errWriter, "Error: failed to open docmap %q: %v\n", *docmapFlag, err)
|
||||||
return 2
|
return 2
|
||||||
}
|
}
|
||||||
defer f.Close() // nolint:errcheck
|
defer func() { _ = f.Close() }()
|
||||||
|
// Verify we opened the same file that was validated — rejects a swap between
|
||||||
|
// the pre-open Lstat and the open call.
|
||||||
|
postStat, err := f.Stat()
|
||||||
|
if err != nil {
|
||||||
|
fmt.Fprintf(errWriter, "Error: failed to stat open docmap %q: %v\n", *docmapFlag, err)
|
||||||
|
return 2
|
||||||
|
}
|
||||||
|
if !os.SameFile(preStat, postStat) {
|
||||||
|
fmt.Fprintf(errWriter, "Error: --docmap %q changed between validation and open\n", *docmapFlag)
|
||||||
|
return 2
|
||||||
|
}
|
||||||
docmapData, err := io.ReadAll(io.LimitReader(f, maxDocmapBytes+1))
|
docmapData, err := io.ReadAll(io.LimitReader(f, maxDocmapBytes+1))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
fmt.Fprintf(errWriter, "Error: failed to read docmap %q: %v\n", *docmapFlag, err)
|
fmt.Fprintf(errWriter, "Error: failed to read docmap %q: %v\n", *docmapFlag, err)
|
||||||
|
|||||||
@@ -687,6 +687,9 @@ func TestValidateDocmapPath_InRepoSymlinkAllowed(t *testing.T) {
|
|||||||
t.Fatalf("expected in-repo symlink to be accepted, got error: %v", err)
|
t.Fatalf("expected in-repo symlink to be accepted, got error: %v", err)
|
||||||
}
|
}
|
||||||
// The returned resolved path must be the real file (not the symlink entry).
|
// The returned resolved path must be the real file (not the symlink entry).
|
||||||
|
// validateDocmapPath calls filepath.EvalSymlinks internally, so the returned
|
||||||
|
// path is always the fully-resolved real path — it can never equal the
|
||||||
|
// symlink entry itself.
|
||||||
if resolved == symlinkPath {
|
if resolved == symlinkPath {
|
||||||
t.Errorf("expected resolved path to differ from symlink path")
|
t.Errorf("expected resolved path to differ from symlink path")
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user