From d10b141f67e444cecb16885f6f09346950f8a256 Mon Sep 17 00:00:00 2001 From: John Elliott Date: Wed, 5 Aug 2026 14:20:41 -0400 Subject: [PATCH 1/7] set decompression size limit for comps.xml and modules.yaml --- pkg/yum/module_stream.go | 7 ++++--- pkg/yum/module_stream_test.go | 18 +++++++++++++++--- pkg/yum/repository.go | 7 ++++--- pkg/yum/repository_test.go | 16 +++++++++++++++- 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/pkg/yum/module_stream.go b/pkg/yum/module_stream.go index 35d0d1d..de40a5a 100644 --- a/pkg/yum/module_stream.go +++ b/pkg/yum/module_stream.go @@ -94,7 +94,7 @@ func (r *Repository) ModuleMDs(ctx context.Context) ([]ModuleMD, int, error) { } defer resp.Body.Close() - if moduleMDs, err = parseModuleMDs(resp.Body); err != nil { + if moduleMDs, err = parseModuleMDs(resp.Body, *r.settings.MaxXmlSize); err != nil { return nil, resp.StatusCode, fmt.Errorf("error parsing comps.xml: %w", err) } @@ -108,7 +108,7 @@ func (r *Repository) ModuleMDs(ctx context.Context) ([]ModuleMD, int, error) { // this breaks parsing into two parts: // 1. use node to read the document type // 2. if the document type is modulemd, fully decode the value -func parseModuleMDs(body io.ReadCloser) ([]ModuleMD, error) { +func parseModuleMDs(body io.ReadCloser, maxSize int64) ([]ModuleMD, error) { moduleMDs := make([]ModuleMD, 0) reader, err := ExtractIfCompressed(body) @@ -118,7 +118,8 @@ func parseModuleMDs(body io.ReadCloser) ([]ModuleMD, error) { yaml.RegisterCustomUnmarshaler[StreamVersion](unmarshalStreamVersion) - decoder := yaml.NewDecoder(reader) + limitedReader := io.LimitReader(reader, maxSize) + decoder := yaml.NewDecoder(limitedReader) for { var node ast.Node err := decoder.Decode(&node) diff --git a/pkg/yum/module_stream_test.go b/pkg/yum/module_stream_test.go index 5f622f6..adf0e65 100644 --- a/pkg/yum/module_stream_test.go +++ b/pkg/yum/module_stream_test.go @@ -13,18 +13,30 @@ func TestParseModuleMDs(t *testing.T) { f, err := os.Open("mocks/module.yaml.zst") assert.NoError(t, err) - parsed, err := parseModuleMDs(f) + parsed, err := parseModuleMDs(f, DefaultMaxXmlSize) assert.NoError(t, err) assert.Equal(t, 13, len(parsed)) assert.NotEmpty(t, parsed[0].Data.Name) assert.NotEmpty(t, parsed[0].Data.Artifacts.Rpms) } +// A maxSize that's smaller than the decompressed modules.yaml must bound how much is read, +// rather than fully decompressing/parsing the payload (decompression-bomb protection). +func TestParseModuleMDsMaxLimit(t *testing.T) { + f, err := os.Open("mocks/module.yaml.zst") + assert.NoError(t, err) + defer f.Close() + + parsed, err := parseModuleMDs(f, 10) + assert.Error(t, err) + assert.Empty(t, parsed) +} + func TestStreamVersionPrecision(t *testing.T) { f, err := os.Open("mocks/module.yaml.zst") assert.NoError(t, err) - parsed, err := parseModuleMDs(f) + parsed, err := parseModuleMDs(f, DefaultMaxXmlSize) assert.NoError(t, err) handlesFloatFound, handlesStringFound := false, false @@ -48,7 +60,7 @@ func TestParseRhel8Modules(t *testing.T) { defer f.Close() require.NoError(t, err) - modules, err := parseModuleMDs(f) + modules, err := parseModuleMDs(f, DefaultMaxXmlSize) require.NoError(t, err) assert.Len(t, modules, 961) diff --git a/pkg/yum/repository.go b/pkg/yum/repository.go index d52ce08..fbd25ec 100644 --- a/pkg/yum/repository.go +++ b/pkg/yum/repository.go @@ -220,7 +220,7 @@ func (r *Repository) Comps(ctx context.Context) (*Comps, int, error) { defer resp.Body.Close() - if comps, err = ParseCompsXML(resp.Body, compsURL); err != nil { + if comps, err = ParseCompsXML(resp.Body, compsURL, *r.settings.MaxXmlSize); err != nil { return nil, resp.StatusCode, fmt.Errorf("error parsing comps.xml: %w", err) } @@ -462,7 +462,7 @@ func ParseRepomdXML(body io.ReadCloser) (Repomd, error) { } // ParseCompsXML creates PackageGroup array and Environment array from comps.xml body response -func ParseCompsXML(body io.ReadCloser, url *string) (Comps, error) { +func ParseCompsXML(body io.ReadCloser, url *string, maxSize int64) (Comps, error) { var reader io.Reader var comps Comps packageGroups := []PackageGroup{} @@ -474,7 +474,8 @@ func ParseCompsXML(body io.ReadCloser, url *string) (Comps, error) { return comps, err } - decoder := xml.NewDecoder(reader) + limitedReader := io.LimitReader(reader, maxSize) + decoder := xml.NewDecoder(limitedReader) for { t, decodeError := decoder.Token() diff --git a/pkg/yum/repository_test.go b/pkg/yum/repository_test.go index bf61eb2..78522f9 100644 --- a/pkg/yum/repository_test.go +++ b/pkg/yum/repository_test.go @@ -303,12 +303,26 @@ func TestParseCompsXML(t *testing.T) { xmlFile, err := os.Open(path) assert.NoError(t, err) defer xmlFile.Close() - comps, err := ParseCompsXML(xmlFile, &path) + comps, err := ParseCompsXML(xmlFile, &path, DefaultMaxXmlSize) assert.NoError(t, err) assert.NotEmpty(t, comps) } } +// A maxSize that's smaller than the decompressed comps.xml must bound how much is read, +// rather than fully decompressing/parsing the payload (decompression-bomb protection). +func TestParseCompsXMLMaxLimit(t *testing.T) { + path := "mocks/comps.xml.gz" + xmlFile, err := os.Open(path) + assert.NoError(t, err) + defer xmlFile.Close() + + comps, err := ParseCompsXML(xmlFile, &path, 10) + assert.Error(t, err) + assert.Empty(t, comps.PackageGroups) + assert.Empty(t, comps.Environments) +} + // if the xml is half complete, you get a parse error func TestParseCompressedXMLDataWithError(t *testing.T) { xmlFile, err := os.Open("mocks/primary.xml.gz") From d37946d9e64c09660836df8d9b72724fcdec17a4 Mon Sep 17 00:00:00 2001 From: John Elliott Date: Wed, 5 Aug 2026 17:03:21 -0400 Subject: [PATCH 2/7] correct modulemds error messaging when decompression limit is exceeded --- pkg/yum/module_stream.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/yum/module_stream.go b/pkg/yum/module_stream.go index de40a5a..d3a33de 100644 --- a/pkg/yum/module_stream.go +++ b/pkg/yum/module_stream.go @@ -94,8 +94,8 @@ func (r *Repository) ModuleMDs(ctx context.Context) ([]ModuleMD, int, error) { } defer resp.Body.Close() - if moduleMDs, err = parseModuleMDs(resp.Body, *r.settings.MaxXmlSize); err != nil { - return nil, resp.StatusCode, fmt.Errorf("error parsing comps.xml: %w", err) + if moduleMDs, err = parseModuleMDs(resp.Body, 10); err != nil { + return nil, resp.StatusCode, fmt.Errorf("error parsing modulemds: %w", err) } return moduleMDs, resp.StatusCode, nil @@ -118,11 +118,16 @@ func parseModuleMDs(body io.ReadCloser, maxSize int64) ([]ModuleMD, error) { yaml.RegisterCustomUnmarshaler[StreamVersion](unmarshalStreamVersion) - limitedReader := io.LimitReader(reader, maxSize) + limitedReader := &io.LimitedReader{R: reader, N: maxSize} decoder := yaml.NewDecoder(limitedReader) for { var node ast.Node err := decoder.Decode(&node) + + if limitedReader.N <= 0 { + return nil, fmt.Errorf("decompression limit of %d bytes met or exceeded", maxSize) + } + if err != nil { if errors.Is(err, io.EOF) { break From 115245f02d19e7ef4561cc56e194a387cdf5fcd6 Mon Sep 17 00:00:00 2001 From: John Elliott Date: Fri, 7 Aug 2026 17:42:30 -0400 Subject: [PATCH 3/7] fix comps error handling --- pkg/yum/repository.go | 22 ++++++++++++++++------ pkg/yum/utils.go | 10 ++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/pkg/yum/repository.go b/pkg/yum/repository.go index fbd25ec..d34252b 100644 --- a/pkg/yum/repository.go +++ b/pkg/yum/repository.go @@ -5,6 +5,7 @@ import ( "compress/gzip" "context" "encoding/xml" + "errors" "fmt" "io" "net/http" @@ -474,18 +475,21 @@ func ParseCompsXML(body io.ReadCloser, url *string, maxSize int64) (Comps, error return comps, err } - limitedReader := io.LimitReader(reader, maxSize) + // Wrap with maxSize + 1 so limit error only triggers when limit is exceeded + limitedReader := io.LimitReader(reader, maxSize+1) decoder := xml.NewDecoder(limitedReader) for { t, decodeError := decoder.Token() - if decodeError == io.EOF { - break - } else if decodeError != nil { + if decodeError != nil { + if limitErr := CheckLimit(limitedReader, maxSize); limitErr != nil { + return comps, limitErr + } + if errors.Is(decodeError, io.EOF) { + break + } return comps, fmt.Errorf("error decoding token: %w", decodeError) - } else if t == nil { - break } switch elType := t.(type) { @@ -494,12 +498,18 @@ func ParseCompsXML(body io.ReadCloser, url *string, maxSize int64) (Comps, error case "group": var packageGroup PackageGroup if decodeElementError := decoder.DecodeElement(&packageGroup, &elType); decodeElementError != nil { + if limitErr := CheckLimit(limitedReader, maxSize); limitErr != nil { + return comps, limitErr + } return comps, decodeElementError } packageGroups = append(packageGroups, packageGroup) case "environment": var environment Environment if decodeElementError := decoder.DecodeElement(&environment, &elType); decodeElementError != nil { + if limitErr := CheckLimit(limitedReader, maxSize); limitErr != nil { + return comps, limitErr + } return comps, decodeElementError } environments = append(environments, environment) diff --git a/pkg/yum/utils.go b/pkg/yum/utils.go index dcb4213..4058290 100644 --- a/pkg/yum/utils.go +++ b/pkg/yum/utils.go @@ -2,6 +2,7 @@ package yum import ( "bufio" + "fmt" "io" "github.com/h2non/filetype" @@ -36,3 +37,12 @@ func ExtractIfCompressed(reader io.ReadCloser) (extractedReader io.Reader, err e return bufferedReader, nil } } + +// CheckLimit inspects an io.Reader (typically returned from io.LimitReader) +// to see if the byte limit has been exceeded. +func CheckLimit(r io.Reader, maxSize int64) error { + if lr, ok := r.(*io.LimitedReader); ok && lr.N == 0 { + return fmt.Errorf("decompression limit of %d bytes exceeded", maxSize) + } + return nil +} From 21be207f18c7a32db92f611888df4624aa28830d Mon Sep 17 00:00:00 2001 From: John Elliott Date: Fri, 7 Aug 2026 17:54:11 -0400 Subject: [PATCH 4/7] fix modulemds decompression limit error handling --- pkg/yum/module_stream.go | 24 +++++++++++++++++------- pkg/yum/module_stream_test.go | 1 + 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/pkg/yum/module_stream.go b/pkg/yum/module_stream.go index d3a33de..c945951 100644 --- a/pkg/yum/module_stream.go +++ b/pkg/yum/module_stream.go @@ -94,7 +94,7 @@ func (r *Repository) ModuleMDs(ctx context.Context) ([]ModuleMD, int, error) { } defer resp.Body.Close() - if moduleMDs, err = parseModuleMDs(resp.Body, 10); err != nil { + if moduleMDs, err = parseModuleMDs(resp.Body, *r.settings.MaxXmlSize); err != nil { return nil, resp.StatusCode, fmt.Errorf("error parsing modulemds: %w", err) } @@ -118,20 +118,23 @@ func parseModuleMDs(body io.ReadCloser, maxSize int64) ([]ModuleMD, error) { yaml.RegisterCustomUnmarshaler[StreamVersion](unmarshalStreamVersion) - limitedReader := &io.LimitedReader{R: reader, N: maxSize} + // Wrap with maxSize + 1 so limit error only triggers when limit is exceeded + limitedReader := io.LimitReader(reader, maxSize+1) decoder := yaml.NewDecoder(limitedReader) + for { var node ast.Node err := decoder.Decode(&node) - if limitedReader.N <= 0 { - return nil, fmt.Errorf("decompression limit of %d bytes met or exceeded", maxSize) - } - if err != nil { + if limitErr := CheckLimit(limitedReader, maxSize); limitErr != nil { + return nil, limitErr + } + if errors.Is(err, io.EOF) { break - } + } + return nil, fmt.Errorf("error decoding streams: %w", err) } @@ -139,12 +142,19 @@ func parseModuleMDs(body io.ReadCloser, maxSize int64) ([]ModuleMD, error) { Document string `yaml:"document"` } if err := yaml.NodeToValue(node, &docType); err != nil { + // Check limit if NodeToValue fails due to an incomplete/truncated AST + if limitErr := CheckLimit(limitedReader, maxSize); limitErr != nil { + return nil, limitErr + } return nil, fmt.Errorf("error decoding document type: %w", err) } if docType.Document == "modulemd" { var module ModuleMD if err := yaml.NodeToValue(node, &module); err != nil { + if limitErr := CheckLimit(limitedReader, maxSize); limitErr != nil { + return nil, limitErr + } return nil, fmt.Errorf("error decoding modulemd: %w", err) } moduleMDs = append(moduleMDs, module) diff --git a/pkg/yum/module_stream_test.go b/pkg/yum/module_stream_test.go index adf0e65..5dde555 100644 --- a/pkg/yum/module_stream_test.go +++ b/pkg/yum/module_stream_test.go @@ -29,6 +29,7 @@ func TestParseModuleMDsMaxLimit(t *testing.T) { parsed, err := parseModuleMDs(f, 10) assert.Error(t, err) + assert.ErrorContains(t, err, "decompression limit of 10 bytes exceeded") assert.Empty(t, parsed) } From 1706de59ae100e812f5702e684f37d1a210c47ac Mon Sep 17 00:00:00 2001 From: John Elliott Date: Mon, 10 Aug 2026 15:17:12 -0400 Subject: [PATCH 5/7] fix lint error --- pkg/yum/module_stream.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/yum/module_stream.go b/pkg/yum/module_stream.go index c945951..b85e7ab 100644 --- a/pkg/yum/module_stream.go +++ b/pkg/yum/module_stream.go @@ -133,8 +133,7 @@ func parseModuleMDs(body io.ReadCloser, maxSize int64) ([]ModuleMD, error) { if errors.Is(err, io.EOF) { break - } - + } return nil, fmt.Errorf("error decoding streams: %w", err) } @@ -143,9 +142,9 @@ func parseModuleMDs(body io.ReadCloser, maxSize int64) ([]ModuleMD, error) { } if err := yaml.NodeToValue(node, &docType); err != nil { // Check limit if NodeToValue fails due to an incomplete/truncated AST - if limitErr := CheckLimit(limitedReader, maxSize); limitErr != nil { - return nil, limitErr - } + if limitErr := CheckLimit(limitedReader, maxSize); limitErr != nil { + return nil, limitErr + } return nil, fmt.Errorf("error decoding document type: %w", err) } From 3866b51dfc4d2fe26e78e6aa35da16cafda3212f Mon Sep 17 00:00:00 2001 From: John Elliott Date: Mon, 10 Aug 2026 16:49:33 -0400 Subject: [PATCH 6/7] minor change --- pkg/yum/repository_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/yum/repository_test.go b/pkg/yum/repository_test.go index 78522f9..6dbccaf 100644 --- a/pkg/yum/repository_test.go +++ b/pkg/yum/repository_test.go @@ -319,6 +319,7 @@ func TestParseCompsXMLMaxLimit(t *testing.T) { comps, err := ParseCompsXML(xmlFile, &path, 10) assert.Error(t, err) + assert.ErrorContains(t, err, "decompression limit of 10 bytes exceeded") assert.Empty(t, comps.PackageGroups) assert.Empty(t, comps.Environments) } From 9f2c96f2afe1a94f46b22162175a01b2f168334c Mon Sep 17 00:00:00 2001 From: John Elliott Date: Thu, 13 Aug 2026 13:14:10 -0400 Subject: [PATCH 7/7] add specific error handling to xml data decompression --- pkg/yum/module_stream_test.go | 2 +- pkg/yum/repository.go | 19 ++++++++++++------- pkg/yum/repository_test.go | 9 +++++---- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/pkg/yum/module_stream_test.go b/pkg/yum/module_stream_test.go index 5dde555..1a1f030 100644 --- a/pkg/yum/module_stream_test.go +++ b/pkg/yum/module_stream_test.go @@ -22,7 +22,7 @@ func TestParseModuleMDs(t *testing.T) { // A maxSize that's smaller than the decompressed modules.yaml must bound how much is read, // rather than fully decompressing/parsing the payload (decompression-bomb protection). -func TestParseModuleMDsMaxLimit(t *testing.T) { +func TestParseModuleMDsMaxLimitError(t *testing.T) { f, err := os.Open("mocks/module.yaml.zst") assert.NoError(t, err) defer f.Close() diff --git a/pkg/yum/repository.go b/pkg/yum/repository.go index d34252b..5e7c9f2 100644 --- a/pkg/yum/repository.go +++ b/pkg/yum/repository.go @@ -580,20 +580,22 @@ func ParseCompressedXMLData(body io.Reader, maxSize int64) ([]Package, error) { return []Package{}, fmt.Errorf("error unzipping response body: %w", err) } - limitedReader := io.LimitReader(reader, maxSize) + // Wrap with maxSize + 1 so limit error only triggers when limit is exceeded + limitedReader := io.LimitReader(reader, maxSize+1) decoder := xml.NewDecoder(limitedReader) for { // Read tokens from the XML document in a stream. t, decodeError := decoder.Token() - // If we are at the end of the file, we are done - if decodeError == io.EOF { - break - } else if decodeError != nil { + if decodeError != nil { + if limitErr := CheckLimit(limitedReader, maxSize); limitErr != nil { + return []Package{}, limitErr + } + if errors.Is(decodeError, io.EOF) { + break + } return []Package{}, fmt.Errorf("error decoding token: %w", decodeError) - } else if t == nil { - break } // Here, we inspect the token @@ -604,6 +606,9 @@ func ParseCompressedXMLData(body io.Reader, maxSize int64) ([]Package, error) { case "package": var pkg Package if decodeElementError := decoder.DecodeElement(&pkg, &elType); decodeElementError != nil { + if limitErr := CheckLimit(limitedReader, maxSize); limitErr != nil { + return []Package{}, limitErr + } return result, decodeElementError } // Ensure that the type is "rpm" before pushing our array diff --git a/pkg/yum/repository_test.go b/pkg/yum/repository_test.go index 6dbccaf..5c4c49f 100644 --- a/pkg/yum/repository_test.go +++ b/pkg/yum/repository_test.go @@ -311,7 +311,7 @@ func TestParseCompsXML(t *testing.T) { // A maxSize that's smaller than the decompressed comps.xml must bound how much is read, // rather than fully decompressing/parsing the payload (decompression-bomb protection). -func TestParseCompsXMLMaxLimit(t *testing.T) { +func TestParseCompsXMLMaxLimitError(t *testing.T) { path := "mocks/comps.xml.gz" xmlFile, err := os.Open(path) assert.NoError(t, err) @@ -325,21 +325,22 @@ func TestParseCompsXMLMaxLimit(t *testing.T) { } // if the xml is half complete, you get a parse error -func TestParseCompressedXMLDataWithError(t *testing.T) { +func TestParseCompressedXMLDataMaxLimitError(t *testing.T) { xmlFile, err := os.Open("mocks/primary.xml.gz") assert.NoError(t, err) defer xmlFile.Close() result, err := ParseCompressedXMLData(xmlFile, 200) assert.Error(t, err) + assert.ErrorContains(t, err, "decompression limit of 200 bytes exceeded") assert.Empty(t, result) } // If no elements are parsed, no error is thrown, but you get empty results -func TestParseCompressedXMLDataMaxLimit(t *testing.T) { +func TestParseCompressedXMLDataNoXMLElements(t *testing.T) { xmlFile, err := os.Open("mocks/aaaa.xml.gz") assert.NoError(t, err) defer xmlFile.Close() - result, err := ParseCompressedXMLData(xmlFile, 10) + result, err := ParseCompressedXMLData(xmlFile, DefaultMaxXmlSize) assert.NoError(t, err) assert.Empty(t, result) }