Skip to content

Commit 6e18d98

Browse files
committed
distribution: prevent log entries created from user input
Add utils.SanitizeForLog which sanitizes a string for safe logging by removing or escaping control characters that could cause log injection attacks. Signed-off-by: Dorin Geman <dorin.geman@docker.com>
1 parent 40c19fa commit 6e18d98

File tree

2 files changed

+54
-7
lines changed

2 files changed

+54
-7
lines changed

pkg/distribution/distribution/client.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"io"
88
"net/http"
99

10+
"github.com/docker/model-runner/pkg/distribution/internal/utils"
1011
"github.com/sirupsen/logrus"
1112

1213
"github.com/docker/model-runner/pkg/distribution/internal/progress"
@@ -134,7 +135,7 @@ func NewClient(opts ...Option) (*Client, error) {
134135

135136
// PullModel pulls a model from a registry and returns the local file path
136137
func (c *Client) PullModel(ctx context.Context, reference string, progressWriter io.Writer) error {
137-
c.log.Infoln("Starting model pull:", reference)
138+
c.log.Infoln("Starting model pull:", utils.SanitizeForLog(reference))
138139

139140
remoteModel, err := c.registry.Model(ctx, reference)
140141
if err != nil {
@@ -157,7 +158,7 @@ func (c *Client) PullModel(ctx context.Context, reference string, progressWriter
157158
// Check if model exists in local store
158159
localModel, err := c.store.Read(remoteDigest.String())
159160
if err == nil {
160-
c.log.Infoln("Model found in local store:", reference)
161+
c.log.Infoln("Model found in local store:", utils.SanitizeForLog(reference))
161162
cfg, err := localModel.Config()
162163
if err != nil {
163164
return fmt.Errorf("getting cached model config: %w", err)
@@ -176,7 +177,7 @@ func (c *Client) PullModel(ctx context.Context, reference string, progressWriter
176177
}
177178
return nil
178179
} else {
179-
c.log.Infoln("Model not found in local store, pulling from remote:", reference)
180+
c.log.Infoln("Model not found in local store, pulling from remote:", utils.SanitizeForLog(reference))
180181
}
181182

182183
// Model doesn't exist in local store or digests don't match, pull from remote
@@ -268,10 +269,10 @@ func (c *Client) ListModels() ([]types.Model, error) {
268269

269270
// GetModel returns a model by reference
270271
func (c *Client) GetModel(reference string) (types.Model, error) {
271-
c.log.Infoln("Getting model by reference:", reference)
272+
c.log.Infoln("Getting model by reference:", utils.SanitizeForLog(reference))
272273
model, err := c.store.Read(reference)
273274
if err != nil {
274-
c.log.Errorln("Failed to get model:", err, "reference:", reference)
275+
c.log.Errorln("Failed to get model:", err, "reference:", utils.SanitizeForLog(reference))
275276
return nil, fmt.Errorf("get model '%q': %w", reference, err)
276277
}
277278

@@ -280,7 +281,7 @@ func (c *Client) GetModel(reference string) (types.Model, error) {
280281

281282
// IsModelInStore checks if a model with the given reference is in the local store
282283
func (c *Client) IsModelInStore(reference string) (bool, error) {
283-
c.log.Infoln("Checking model by reference:", reference)
284+
c.log.Infoln("Checking model by reference:", utils.SanitizeForLog(reference))
284285
if _, err := c.store.Read(reference); errors.Is(err, ErrModelNotFound) {
285286
return false, nil
286287
} else if err != nil {
@@ -349,7 +350,7 @@ func (c *Client) DeleteModel(reference string, force bool) (*DeleteModelResponse
349350

350351
// Tag adds a tag to a model
351352
func (c *Client) Tag(source string, target string) error {
352-
c.log.Infoln("Tagging model, source:", source, "target:", target)
353+
c.log.Infoln("Tagging model, source:", source, "target:", utils.SanitizeForLog(target))
353354
return c.store.AddTags(source, []string{target})
354355
}
355356

pkg/distribution/internal/utils/utils.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"net/url"
88
"os"
99
"strings"
10+
"unicode"
1011
)
1112

1213
// FormatBytes converts bytes to a human-readable string with appropriate unit
@@ -103,3 +104,48 @@ func (pr *ProgressReader) Read(p []byte) (int, error) {
103104
}
104105
return n, err
105106
}
107+
108+
// SanitizeForLog sanitizes a string for safe logging by removing or escaping
109+
// control characters that could cause log injection attacks.
110+
// TODO: Consider migrating to structured logging which
111+
// handles sanitization automatically through field encoding.
112+
func SanitizeForLog(s string) string {
113+
if s == "" {
114+
return ""
115+
}
116+
117+
var result strings.Builder
118+
result.Grow(len(s))
119+
120+
for _, r := range s {
121+
switch {
122+
// Replace newlines and carriage returns with escaped versions.
123+
case r == '\n':
124+
result.WriteString("\\n")
125+
case r == '\r':
126+
result.WriteString("\\r")
127+
case r == '\t':
128+
result.WriteString("\\t")
129+
// Remove other control characters (0x00-0x1F, 0x7F).
130+
case unicode.IsControl(r):
131+
// Skip control characters or replace with placeholder.
132+
result.WriteString("?")
133+
// Escape backslashes to prevent escape sequence injection.
134+
case r == '\\':
135+
result.WriteString("\\\\")
136+
// Keep printable characters.
137+
case unicode.IsPrint(r):
138+
result.WriteRune(r)
139+
default:
140+
// Replace non-printable characters with placeholder.
141+
result.WriteString("?")
142+
}
143+
}
144+
145+
const maxLength = 100
146+
if result.Len() > maxLength {
147+
return result.String()[:maxLength] + "...[truncated]"
148+
}
149+
150+
return result.String()
151+
}

0 commit comments

Comments
 (0)