diff --git a/modules/brother/README.md b/modules/brother/README.md index f99bddea..2cf6d4f0 100644 --- a/modules/brother/README.md +++ b/modules/brother/README.md @@ -75,7 +75,7 @@ makes it (mode 0755) when it files the first scan. |---|---| | `printer.status` | without a login: over IPP (`Get-Printer-Attributes`, encoded by hand, on port 631) the state (idle, processing, stopped), the state reasons (such as `toner-low-warning`) and the markers' names, levels (percent; below 0 is unknown) and colours; over eSCL the scanner's state and its feeder's. A part that does not answer is `answers: false` with why. Needs `printer` and `scanner` | | `printer.upload` | one part of a document to print later (`upload` id, `part`, `parts`, `sha256` of the whole, `data` base64): stored as `uploads/.pdf` or `.jpg` once all parts came and the sha256 matches; a wrong sha256, a part out of range or a document that is neither PDF nor JPEG stores nothing | -| `printer.print` | prints a file on the machine the tool runs on (`path`, absolute: the tool reads it, uploads it in parts and prints it in one call; PDF or JPEG by its bytes, never its name), a kept scan (`object`) or a completed upload (`upload`, for modules that send bytes over the bus) as an IPP Print-Job (operation 0x0002, port 631) in its own format, `copies` 1 to 99 (default 1), `sides` one-sided, two-sided-long-edge or two-sided-short-edge (default one-sided), `media` (default `iso_a4_210x297mm`); answers the job's id and state (with `path`, the upload id and the size when `path` was given) | +| `printer.print` | prints a file on the machine the tool runs on (`path`, absolute, a regular file under the tool account's home folder after symbolic links are resolved; any other path is refused with the one message "path refused: only a file under the home folder", which says nothing of whether it exists: the tool reads it, uploads it in parts and prints it in one call; PDF or JPEG by its bytes, never its name), a kept scan (`object`) or a completed upload (`upload`, for modules that send bytes over the bus) as an IPP Print-Job (operation 0x0002, port 631) in its own format, `copies` 1 to 99 (default 1), `sides` one-sided, two-sided-long-edge or two-sided-short-edge (default one-sided), `media` (default `iso_a4_210x297mm`); answers the job's id and state (with `path`, the upload id and the size when `path` was given) | | `printer.settings` | asks the device process (below) to log in, read one admin page and log out, and waits up to 60 s for its answer. Argument `page`: a path on the admin site, starting with `/` and ending in `.html` (default `/general/status.html`). Answers the page's title and forms with their fields, each with its current value (a radio group as one field with the checked button's value, a checkbox with `checked`, a select with the selected option) and `options`; the page's links only with `links: true`, hidden fields only with `hidden: true`; a password field's value is never given. Read-only: it sends the login and the logout and nothing else | ### Trust and the password diff --git a/modules/brother/cmd/brother/documents_test.go b/modules/brother/cmd/brother/documents_test.go index f0f2604b..a876c540 100644 --- a/modules/brother/cmd/brother/documents_test.go +++ b/modules/brother/cmd/brother/documents_test.go @@ -540,13 +540,21 @@ func TestOnlyATooLargeAnswerEndsTheAskATransientFailureIsRetried(t *testing.T) { } } +// home makes a home folder for the tool account and returns it. +func home(t *testing.T) string { + t.Helper() + h := t.TempDir() + t.Setenv("HOME", h) + return h +} + // 1551: printer.print with a path reads the file, uploads it in parts and prints it in one call; a document of // several parts arrives whole, and the answer has the job's id and state. func TestPrinterPrintTakesAPathAndUploadsAndPrintsInOneCall(t *testing.T) { fake := newIPPFake(t, 31, 5) r := newRig(t, `{"printer":"192.0.2.10"}`, "pw") doc := pdfOf(2*PartSize + 1234) - file := filepath.Join(t.TempDir(), "letter.bin") // the name says nothing: the bytes are read + file := filepath.Join(home(t), "letter.bin") // the name says nothing: the bytes are read if err := os.WriteFile(file, doc, 0o600); err != nil { t.Fatal(err) } @@ -568,29 +576,54 @@ func TestPrinterPrintTakesAPathAndUploadsAndPrintsInOneCall(t *testing.T) { } } -// 1551: a file that is neither a PDF nor a JPEG by its bytes is refused whatever its name, and a path that is -// relative, missing, a folder, or named together with an object is refused; nothing is queued. -func TestPrinterPrintRefusesAPathThatIsNotAPDFOrJPEG(t *testing.T) { +// 1551: only a regular file under the tool account's home folder is read, after symbolic links are resolved; every +// other path is refused in one fixed way that says nothing of whether it exists or how large it is; a file in the +// home folder that is no PDF or JPEG by its bytes is refused by name of the type. +func TestPrinterPrintReadsOnlyAFileUnderTheHomeFolder(t *testing.T) { fake := newIPPFake(t, 1, 5) r := newRig(t, `{"printer":"192.0.2.10"}`, "pw") - dir := t.TempDir() - text := filepath.Join(dir, "fake.pdf") - os.WriteFile(text, []byte("just text, named .pdf"), 0o600) - for name, args := range map[string]map[string]any{ - "text named pdf": {"path": text}, - "relative": {"path": "fake.pdf"}, - "missing": {"path": filepath.Join(dir, "none.pdf")}, - "folder": {"path": dir}, - "with object": {"path": text, "object": sha256Hex([]byte("x")) + ".pdf"}, + h := home(t) + outside := t.TempDir() + secret := filepath.Join(outside, "secret.pdf") + os.WriteFile(secret, pdfOf(3000), 0o600) + link := filepath.Join(h, "link.pdf") + os.Symlink(secret, link) + dirLink := filepath.Join(h, "dirlink") + os.Symlink(outside, dirLink) + os.Mkdir(filepath.Join(h, "folder"), 0o700) + for name, path := range map[string]string{ + "outside home": secret, + "link out of home": link, + "folder link out": filepath.Join(dirLink, "secret.pdf"), + "dot-dot out of home": filepath.Join(h, "..", filepath.Base(outside), "secret.pdf"), + "missing in home": filepath.Join(h, "none.pdf"), + "missing outside home": filepath.Join(outside, "none.pdf"), + "folder": filepath.Join(h, "folder"), + "the home folder itself": h, + "relative": "secret.pdf", + "proc": "/proc/self/environ", } { - if _, err := r.call(t, "printer.print", args); err == nil { - t.Errorf("%s: printed", name) + _, err := r.call(t, "printer.print", map[string]any{"path": path}) + if err == nil || err.Error() != PathRefused { + t.Errorf("%s: error = %v, want exactly %q", name, err, PathRefused) } } + text := filepath.Join(h, "fake.pdf") + os.WriteFile(text, []byte("just text, named .pdf"), 0o600) if _, err := r.call(t, "printer.print", map[string]any{"path": text}); err == nil || !strings.Contains(err.Error(), "neither a PDF nor a JPEG") { - t.Errorf("error = %v", err) + t.Errorf("text in home: error = %v", err) + } + if _, err := r.call(t, "printer.print", map[string]any{"path": text, "object": sha256Hex([]byte("x")) + ".pdf"}); err == nil { + t.Error("path with object accepted") } if r.bus.asks() != 0 || fake.body != nil { t.Errorf("%d asks queued, printer sent %d bytes", r.bus.asks(), len(fake.body)) } + // A symbolic link inside the home folder to a file inside it is fine. + inside := filepath.Join(h, "real.pdf") + os.WriteFile(inside, pdfOf(3000), 0o600) + os.Symlink(inside, filepath.Join(h, "ok.pdf")) + if _, err := r.call(t, "printer.print", map[string]any{"path": filepath.Join(h, "ok.pdf")}); err != nil { + t.Errorf("a link within home: %v", err) + } } diff --git a/modules/brother/cmd/brother/file.go b/modules/brother/cmd/brother/file.go index a5f0cc56..3e68324d 100644 --- a/modules/brother/cmd/brother/file.go +++ b/modules/brother/cmd/brother/file.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "os" "path/filepath" @@ -123,21 +124,40 @@ func List(folder string, limit int) ([]Entry, error) { return out, nil } -// ReadDocument is the file at path, read whole: a regular file of at most most bytes. +// PathRefused is the one answer to a path that may not be printed from: it says nothing of whether the path exists, +// what it is or how large (issue 1551). +const PathRefused = "path refused: only a file under the home folder" + +// ReadDocument is the file at path, read whole: a regular file under the tool account's home folder, of at most +// most bytes. The path is resolved through its symbolic links first and the real path must still lie under the +// home folder (which is itself resolved); anything else, a missing path included, is PathRefused and no more. func ReadDocument(path string, most int64) ([]byte, error) { - info, err := os.Stat(path) - if err != nil { - return nil, fmt.Errorf("path %s cannot be read: %w", path, err) + home, err := os.UserHomeDir() + if err != nil || home == "" || !filepath.IsAbs(path) { + return nil, errors.New(PathRefused) } - if !info.Mode().IsRegular() { - return nil, fmt.Errorf("path %s is not a file", path) + realHome, err := filepath.EvalSymlinks(home) + if err != nil { + return nil, errors.New(PathRefused) + } + real, err := filepath.EvalSymlinks(path) + if err != nil { + return nil, errors.New(PathRefused) + } + rel, err := filepath.Rel(realHome, real) + if err != nil || rel == "." || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return nil, errors.New(PathRefused) + } + info, err := os.Stat(real) + if err != nil || !info.Mode().IsRegular() { + return nil, errors.New(PathRefused) } if info.Size() > most { return nil, fmt.Errorf("file %s is %d bytes, over the %d the module takes", path, info.Size(), most) } - doc, err := os.ReadFile(path) + doc, err := os.ReadFile(real) if err != nil { - return nil, fmt.Errorf("path %s cannot be read: %w", path, err) + return nil, fmt.Errorf("file %s cannot be read: %w", path, err) } return doc, nil } diff --git a/modules/brother/cmd/brother/main.go b/modules/brother/cmd/brother/main.go index ff5bdabd..57089e80 100644 --- a/modules/brother/cmd/brother/main.go +++ b/modules/brother/cmd/brother/main.go @@ -131,12 +131,12 @@ func tools(settingsFile string, now func() time.Time) []stdio.Tool { }, { Name: "printer.print", - Description: "Print a document on the printer: a file on the machine this tool runs on (path: reads it, uploads it in parts " + + Description: "Print a document on the printer: a file under the home folder of the account the tool runs as, on the machine it runs on (path: reads it, uploads it in parts " + "and prints it, in one call), a scan kept in the bucket (object), or a completed upload (upload; for a module that sends " + "bytes over the bus). Only PDF and JPEG, told by the file's bytes and not its name. The device " + "process sends it as an IPP Print-Job in its own format and answers the job's id and state (with path, the upload id and the size).", Input: map[string]any{"type": "object", "properties": map[string]any{ - "path": str("an absolute path of a PDF or JPEG file on the machine the tool runs on"), + "path": str("an absolute path of a PDF or JPEG file under the tool account's home folder (symbolic links are resolved and the real path must lie there too; any other path is refused in one fixed way)"), "object": str("a scan's key, as scan.added names it"), "upload": str("a completed upload's id"), "copies": num("1 to 99 (default 1)"), diff --git a/modules/brother/cmd/brother/printer.go b/modules/brother/cmd/brother/printer.go index f2bc4a58..a7acc06e 100644 --- a/modules/brother/cmd/brother/printer.go +++ b/modules/brother/cmd/brother/printer.go @@ -7,7 +7,6 @@ import ( "encoding/hex" "encoding/json" "fmt" - "path/filepath" "regexp" "slices" "strings" @@ -316,8 +315,10 @@ func (s *Service) PrintFile(ctx context.Context, path string, a Ask) (map[string if a.Object != "" || a.Upload != "" { return nil, fmt.Errorf("name path, or object, or upload: one of the three") } - if !filepath.IsAbs(path) { - return nil, fmt.Errorf("path %q is not an absolute path on the machine the tool runs on", path) + // The path is checked first, and refused in one fixed way that tells nothing about the file. + doc, err := ReadDocument(path, MostParts*PartSize) + if err != nil { + return nil, err } a.Upload = "p-" + newAskID() if err := checkPrint(&a); err != nil { @@ -326,10 +327,6 @@ func (s *Service) PrintFile(ctx context.Context, path string, a Ask) (map[string if s.Settings.Printer == "" { return nil, unsetError("printer") } - doc, err := ReadDocument(path, MostParts*PartSize) - if err != nil { - return nil, err - } if _, _, err := SniffFormat(doc); err != nil { return nil, fmt.Errorf("file %s: %w", path, err) }