From 87c7c80e0ab71366c34869d05f9b5a4fc4667432 Mon Sep 17 00:00:00 2001
From: Hidde Beydals <hello@hidde.co>
Date: Tue, 23 Nov 2021 09:30:37 +0100
Subject: [PATCH 1/2] internal/helm: validate package while loading meta

There was an unfinished code path that should have continued validating
the paths within the package. This commit completes it.

Signed-off-by: Hidde Beydals <hello@hidde.co>
---
 internal/helm/chart/metadata.go | 38 +++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/internal/helm/chart/metadata.go b/internal/helm/chart/metadata.go
index f59a599b..673ee0ae 100644
--- a/internal/helm/chart/metadata.go
+++ b/internal/helm/chart/metadata.go
@@ -28,6 +28,7 @@ import (
 	"path"
 	"path/filepath"
 	"reflect"
+	"regexp"
 	"strings"
 
 	helmchart "helm.sh/helm/v3/pkg/chart"
@@ -37,6 +38,8 @@ import (
 	"github.com/fluxcd/source-controller/internal/helm"
 )
 
+var drivePathPattern = regexp.MustCompile(`^[a-zA-Z]:/`)
+
 // OverwriteChartDefaultValues overwrites the chart default values file with the given data.
 func OverwriteChartDefaultValues(chart *helmchart.Chart, vals chartutil.Values) (bool, error) {
 	if vals == nil {
@@ -161,6 +164,9 @@ func LoadChartMetadataFromArchive(archive string) (*helmchart.Metadata, error) {
 	}
 	tr := tar.NewReader(zr)
 
+	// The following logic is on par with how Helm validates the package while
+	// unpackaging it, except that we only read the Metadata related files.
+	// Ref: https://github.com/helm/helm/blob/a499b4b179307c267bdf3ec49b880e3dbd2a5591/pkg/chart/loader/archive.go#L104
 	var m *helmchart.Metadata
 	for {
 		hd, err := tr.Next()
@@ -189,17 +195,37 @@ func LoadChartMetadataFromArchive(archive string) (*helmchart.Metadata, error) {
 			delimiter = "\\"
 		}
 		parts := strings.Split(hd.Name, delimiter)
+		n := strings.Join(parts[1:], delimiter)
 
-		// We are only interested in files in the base directory
+		// Normalize the path to the / delimiter
+		n = strings.ReplaceAll(n, delimiter, "/")
+
+		if path.IsAbs(n) {
+			return nil, errors.New("chart illegally contains absolute paths")
+		}
+
+		n = path.Clean(n)
+		if n == "." {
+			// In this case, the original path was relative when it should have been absolute.
+			return nil, fmt.Errorf("chart illegally contains content outside the base directory: %s", hd.Name)
+		}
+		if strings.HasPrefix(n, "..") {
+			return nil, fmt.Errorf("chart illegally references parent directory")
+		}
+
+		// In some particularly arcane acts of path creativity, it is possible to intermix
+		// UNIX and Windows style paths in such a way that you produce a result of the form
+		// c:/foo even after all the built-in absolute path checks. So we explicitly check
+		// for this condition.
+		if drivePathPattern.MatchString(n) {
+			return nil, errors.New("chart contains illegally named files")
+		}
+
+		// We are only interested in files in the base directory from here on
 		if len(parts) != 2 {
 			continue
 		}
 
-		// Normalize the path to the / delimiter
-		n := strings.Join(parts[1:], delimiter)
-		n = strings.ReplaceAll(n, delimiter, "/")
-		n = path.Clean(n)
-
 		switch parts[1] {
 		case chartutil.ChartfileName, "requirements.yaml":
 			b, err := io.ReadAll(tr)

From ee1cb49b0c3744b2417092883a35545f307e6dc9 Mon Sep 17 00:00:00 2001
From: Hidde Beydals <hello@hidde.co>
Date: Tue, 23 Nov 2021 09:33:04 +0100
Subject: [PATCH 2/2] internal/helm: check size of meta files in package

Signed-off-by: Hidde Beydals <hello@hidde.co>
---
 internal/helm/chart/metadata.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/internal/helm/chart/metadata.go b/internal/helm/chart/metadata.go
index 673ee0ae..e3c91ac6 100644
--- a/internal/helm/chart/metadata.go
+++ b/internal/helm/chart/metadata.go
@@ -228,6 +228,9 @@ func LoadChartMetadataFromArchive(archive string) (*helmchart.Metadata, error) {
 
 		switch parts[1] {
 		case chartutil.ChartfileName, "requirements.yaml":
+			if hd.Size > helm.MaxChartFileSize {
+				return nil, fmt.Errorf("size of '%s' exceeds '%d' bytes limit", hd.Name, helm.MaxChartFileSize)
+			}
 			b, err := io.ReadAll(tr)
 			if err != nil {
 				return nil, err