bound imageprocessor.Process input read to prevent unbounded memory use
ImageProcessor.Process used io.ReadAll without a size limit, allowing arbitrarily large inputs to exhaust memory. Add a configurable maxInputBytes limit (default 50 MiB, matching the fetcher limit) and reject inputs that exceed it with ErrInputDataTooLarge. Also bound the cached source content read in the service layer to prevent unexpectedly large cached files from consuming unbounded memory. Extracted loadCachedSource helper to reduce nesting complexity.
This commit is contained in:
@@ -15,13 +15,14 @@ import (
|
||||
|
||||
// Service implements the ImageCache interface, orchestrating cache, fetcher, and processor.
|
||||
type Service struct {
|
||||
cache *Cache
|
||||
fetcher Fetcher
|
||||
processor Processor
|
||||
signer *Signer
|
||||
whitelist *HostWhitelist
|
||||
log *slog.Logger
|
||||
allowHTTP bool
|
||||
cache *Cache
|
||||
fetcher Fetcher
|
||||
processor Processor
|
||||
signer *Signer
|
||||
whitelist *HostWhitelist
|
||||
log *slog.Logger
|
||||
allowHTTP bool
|
||||
maxResponseSize int64
|
||||
}
|
||||
|
||||
// ServiceConfig holds configuration for the image service.
|
||||
@@ -50,15 +51,17 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
|
||||
return nil, errors.New("signing key is required")
|
||||
}
|
||||
|
||||
// Resolve fetcher config for defaults
|
||||
fetcherCfg := cfg.FetcherConfig
|
||||
if fetcherCfg == nil {
|
||||
fetcherCfg = DefaultFetcherConfig()
|
||||
}
|
||||
|
||||
// Use custom fetcher if provided, otherwise create HTTP fetcher
|
||||
var fetcher Fetcher
|
||||
if cfg.Fetcher != nil {
|
||||
fetcher = cfg.Fetcher
|
||||
} else {
|
||||
fetcherCfg := cfg.FetcherConfig
|
||||
if fetcherCfg == nil {
|
||||
fetcherCfg = DefaultFetcherConfig()
|
||||
}
|
||||
fetcher = NewHTTPFetcher(fetcherCfg)
|
||||
}
|
||||
|
||||
@@ -74,14 +77,17 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
|
||||
allowHTTP = cfg.FetcherConfig.AllowHTTP
|
||||
}
|
||||
|
||||
maxResponseSize := fetcherCfg.MaxResponseSize
|
||||
|
||||
return &Service{
|
||||
cache: cfg.Cache,
|
||||
fetcher: fetcher,
|
||||
processor: NewImageProcessor(),
|
||||
signer: signer,
|
||||
whitelist: NewHostWhitelist(cfg.Whitelist),
|
||||
log: log,
|
||||
allowHTTP: allowHTTP,
|
||||
cache: cfg.Cache,
|
||||
fetcher: fetcher,
|
||||
processor: NewImageProcessor(maxResponseSize),
|
||||
signer: signer,
|
||||
whitelist: NewHostWhitelist(cfg.Whitelist),
|
||||
log: log,
|
||||
allowHTTP: allowHTTP,
|
||||
maxResponseSize: maxResponseSize,
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -146,6 +152,40 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
|
||||
return response, nil
|
||||
}
|
||||
|
||||
// loadCachedSource attempts to load source content from cache, returning nil
|
||||
// if the cached data is unavailable or exceeds maxResponseSize.
|
||||
func (s *Service) loadCachedSource(contentHash ContentHash) []byte {
|
||||
reader, err := s.cache.GetSourceContent(contentHash)
|
||||
if err != nil {
|
||||
s.log.Warn("failed to load cached source, fetching", "error", err)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// Bound the read to maxResponseSize to prevent unbounded memory use
|
||||
// from unexpectedly large cached files.
|
||||
limited := io.LimitReader(reader, s.maxResponseSize+1)
|
||||
data, err := io.ReadAll(limited)
|
||||
_ = reader.Close()
|
||||
|
||||
if err != nil {
|
||||
s.log.Warn("failed to read cached source, fetching", "error", err)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
if int64(len(data)) > s.maxResponseSize {
|
||||
s.log.Warn("cached source exceeds max response size, discarding",
|
||||
"hash", contentHash,
|
||||
"max_bytes", s.maxResponseSize,
|
||||
)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
return data
|
||||
}
|
||||
|
||||
// processFromSourceOrFetch processes an image, using cached source content if available.
|
||||
func (s *Service) processFromSourceOrFetch(
|
||||
ctx context.Context,
|
||||
@@ -162,22 +202,8 @@ func (s *Service) processFromSourceOrFetch(
|
||||
var fetchBytes int64
|
||||
|
||||
if contentHash != "" {
|
||||
// We have cached source - load it
|
||||
s.log.Debug("using cached source", "hash", contentHash)
|
||||
|
||||
reader, err := s.cache.GetSourceContent(contentHash)
|
||||
if err != nil {
|
||||
s.log.Warn("failed to load cached source, fetching", "error", err)
|
||||
// Fall through to fetch
|
||||
} else {
|
||||
sourceData, err = io.ReadAll(reader)
|
||||
_ = reader.Close()
|
||||
|
||||
if err != nil {
|
||||
s.log.Warn("failed to read cached source, fetching", "error", err)
|
||||
// Fall through to fetch
|
||||
}
|
||||
}
|
||||
sourceData = s.loadCachedSource(contentHash)
|
||||
}
|
||||
|
||||
// Fetch from upstream if we don't have source data or it's empty
|
||||
|
||||
Reference in New Issue
Block a user