Bound download expansion and escape control chars on the terminal (closes #164)
Objects fetched from the store are untrusted; several decode paths let one expand or print without limit. - blobgen.LimitReader errors past a byte cap (not io.LimitReader silent EOF). DecodeManifest reads through caps on both compressed input and decompressed output, far above any real manifest, so json.Decode cannot buffer a compressible bomb. FetchAndDecryptBlob bounds decompression to the blob recorded uncompressed_size (not the restoring host blob_size_limit). - downloadSnapshotDB streams straight from storage to its temp file with io.Copy, replacing two ReadAll calls that held the whole database twice. - FetchBlob drops the per-blob Stat round-trip, its expectedSize parameter and returned size, all of which only fed a debug log. - TTYHandler and ui.Writer escape control characters in messages, attribute keys/values, and rendered identifiers/paths before colour codes are applied, so a crafted value cannot drive the terminal. Model: opus-4-8
This commit was merged in pull request #197.
This commit is contained in:
@@ -0,0 +1,39 @@
|
||||
package log_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"log/slog"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/vaultik/internal/log"
|
||||
)
|
||||
|
||||
// TestTTYHandlerEscapesControlCharacters logs a message and an attribute
|
||||
// value that each carry an ESC and a newline — the shape a crafted path or
|
||||
// storage error from the destination would take — and checks neither raw
|
||||
// byte reaches the output. The handler's own colour codes (ESC ... m) are
|
||||
// stripped first; any ESC left after that came from the untrusted value.
|
||||
func TestTTYHandlerEscapesControlCharacters(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var buf bytes.Buffer
|
||||
|
||||
logger := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions()))
|
||||
logger.Info("start\x1b[31mZAP\nend", "target", "a\x1b[31mZAP\nb")
|
||||
|
||||
out := buf.String()
|
||||
|
||||
// The only newline is the line terminator; the injected ones were escaped.
|
||||
require.Equal(t, 1, strings.Count(out, "\n"),
|
||||
"a newline in the message or a value must be escaped, not emitted raw")
|
||||
|
||||
// After the handler's own colour codes are removed, no ESC survives.
|
||||
stripped := ansiEscape.ReplaceAllString(out, "")
|
||||
require.NotContains(t, stripped, "\x1b",
|
||||
"a raw ESC from the message or a value must not reach the terminal")
|
||||
|
||||
// The escaped form is what appears instead.
|
||||
require.Contains(t, out, `\x1b`)
|
||||
}
|
||||
@@ -5,9 +5,11 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
"unicode"
|
||||
)
|
||||
|
||||
// groupSeparator joins an open group path to an attribute key. This
|
||||
@@ -116,11 +118,14 @@ func (h *TTYHandler) Handle(_ context.Context, r slog.Record) error {
|
||||
levelColor = colorReset
|
||||
}
|
||||
|
||||
// Print main message
|
||||
// Print main message. The message is escaped before the colour codes
|
||||
// are written around it: it can carry text from an untrusted source
|
||||
// (a storage error, for one), and a raw control character would
|
||||
// otherwise reach the terminal.
|
||||
_, _ = fmt.Fprintf(h.out, "%s%s%s %s%s%s %s%s%s",
|
||||
colorGray, timestamp, colorReset,
|
||||
levelColor, level, colorReset,
|
||||
colorBold, r.Message, colorReset)
|
||||
colorBold, sanitize(r.Message), colorReset)
|
||||
|
||||
// Attributes carried by the handler come first, then the record's
|
||||
// own. Handler attributes were qualified when they were added; the
|
||||
@@ -260,9 +265,29 @@ func (h *TTYHandler) writeAttr(a slog.Attr) {
|
||||
// Future kinds also use the plain string form.
|
||||
}
|
||||
|
||||
// Escape the key and value before the colour codes are written around
|
||||
// them. Both can carry text from an untrusted source — a manifest
|
||||
// timestamp, a storage error, a path or symlink target read back from
|
||||
// the snapshot database — so a control character in one of them must
|
||||
// be rendered as an escape sequence rather than reaching the terminal,
|
||||
// where it could move the cursor or inject its own colours.
|
||||
_, _ = fmt.Fprintf(h.out, " %s%s%s=%s%s%s",
|
||||
colorCyan, a.Key, colorReset,
|
||||
colorBlue, value, colorReset)
|
||||
colorCyan, sanitize(a.Key), colorReset,
|
||||
colorBlue, sanitize(value), colorReset)
|
||||
}
|
||||
|
||||
// sanitize returns s unchanged when every rune in it is printable, and a
|
||||
// double-quoted, backslash-escaped form (\n, \x1b, …) otherwise. It is
|
||||
// applied to untrusted text before any colour code is written, so a
|
||||
// control character can never reach the terminal raw.
|
||||
func sanitize(s string) string {
|
||||
for _, r := range s {
|
||||
if !unicode.IsPrint(r) {
|
||||
return strconv.Quote(s)
|
||||
}
|
||||
}
|
||||
|
||||
return s
|
||||
}
|
||||
|
||||
// formatDuration formats a duration in a human-readable way
|
||||
|
||||
Reference in New Issue
Block a user