Skip to content

Commit 87c7c80

Browse files
committed
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>
1 parent 131c074 commit 87c7c80

File tree

1 file changed

+32
-6
lines changed

1 file changed

+32
-6
lines changed

internal/helm/chart/metadata.go

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"path"
2929
"path/filepath"
3030
"reflect"
31+
"regexp"
3132
"strings"
3233

3334
helmchart "helm.sh/helm/v3/pkg/chart"
@@ -37,6 +38,8 @@ import (
3738
"github.com/fluxcd/source-controller/internal/helm"
3839
)
3940

41+
var drivePathPattern = regexp.MustCompile(`^[a-zA-Z]:/`)
42+
4043
// OverwriteChartDefaultValues overwrites the chart default values file with the given data.
4144
func OverwriteChartDefaultValues(chart *helmchart.Chart, vals chartutil.Values) (bool, error) {
4245
if vals == nil {
@@ -161,6 +164,9 @@ func LoadChartMetadataFromArchive(archive string) (*helmchart.Metadata, error) {
161164
}
162165
tr := tar.NewReader(zr)
163166

167+
// The following logic is on par with how Helm validates the package while
168+
// unpackaging it, except that we only read the Metadata related files.
169+
// Ref: https://github.com/helm/helm/blob/a499b4b179307c267bdf3ec49b880e3dbd2a5591/pkg/chart/loader/archive.go#L104
164170
var m *helmchart.Metadata
165171
for {
166172
hd, err := tr.Next()
@@ -189,16 +195,36 @@ func LoadChartMetadataFromArchive(archive string) (*helmchart.Metadata, error) {
189195
delimiter = "\\"
190196
}
191197
parts := strings.Split(hd.Name, delimiter)
192-
193-
// We are only interested in files in the base directory
194-
if len(parts) != 2 {
195-
continue
196-
}
198+
n := strings.Join(parts[1:], delimiter)
197199

198200
// Normalize the path to the / delimiter
199-
n := strings.Join(parts[1:], delimiter)
200201
n = strings.ReplaceAll(n, delimiter, "/")
202+
203+
if path.IsAbs(n) {
204+
return nil, errors.New("chart illegally contains absolute paths")
205+
}
206+
201207
n = path.Clean(n)
208+
if n == "." {
209+
// In this case, the original path was relative when it should have been absolute.
210+
return nil, fmt.Errorf("chart illegally contains content outside the base directory: %s", hd.Name)
211+
}
212+
if strings.HasPrefix(n, "..") {
213+
return nil, fmt.Errorf("chart illegally references parent directory")
214+
}
215+
216+
// In some particularly arcane acts of path creativity, it is possible to intermix
217+
// UNIX and Windows style paths in such a way that you produce a result of the form
218+
// c:/foo even after all the built-in absolute path checks. So we explicitly check
219+
// for this condition.
220+
if drivePathPattern.MatchString(n) {
221+
return nil, errors.New("chart contains illegally named files")
222+
}
223+
224+
// We are only interested in files in the base directory from here on
225+
if len(parts) != 2 {
226+
continue
227+
}
202228

203229
switch parts[1] {
204230
case chartutil.ChartfileName, "requirements.yaml":

0 commit comments

Comments
 (0)