From 85489c63b4a3c2148229e5384af9e01e4e8471aa Mon Sep 17 00:00:00 2001
From: wyike <wangyike.wyk@alibaba-inc.com>
Date: Fri, 4 Nov 2022 20:15:20 +0800
Subject: [PATCH] Fix: forbid 302 request to avoid SSRF (#5000)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix helm chart list endpoint SSRF CVE

Signed-off-by: 楚岳 <wangyike.wyk@alibaba-inc.com>

* revert error log

Signed-off-by: 楚岳 <wangyike.wyk@alibaba-inc.com>

* change with const value

Signed-off-by: 楚岳 <wangyike.wyk@alibaba-inc.com>

fix ci

Signed-off-by: 楚岳 <wangyike.wyk@alibaba-inc.com>

Signed-off-by: 楚岳 <wangyike.wyk@alibaba-inc.com>
---
 pkg/utils/common/common.go      | 12 +++++++++---
 pkg/utils/common/common_test.go | 20 ++++++++++++++++++++
 pkg/utils/helm/helm_helper.go   |  2 +-
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/pkg/utils/common/common.go b/pkg/utils/common/common.go
index 35aa1031b..63ae82cbe 100644
--- a/pkg/utils/common/common.go
+++ b/pkg/utils/common/common.go
@@ -80,9 +80,15 @@ import (
 var (
 	// Scheme defines the default KubeVela schema
 	Scheme = k8sruntime.NewScheme()
+	// forbidRedirectFunc general check func for http redirect response
+	forbidRedirectFunc = func(req *http.Request, via []*http.Request) error {
+		return errors.New("got a redirect response which is forbidden")
+	}
 	//nolint:gosec
 	// insecureHTTPClient insecure http client
-	insecureHTTPClient = &http.Client{Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}}
+	insecureHTTPClient = &http.Client{Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}, CheckRedirect: forbidRedirectFunc}
+	// forbidRedirectClient is a http client forbid redirect http request
+	forbidRedirectClient = &http.Client{CheckRedirect: forbidRedirectFunc}
 )
 
 const (
@@ -170,7 +176,7 @@ func HTTPGetResponse(ctx context.Context, url string, opts *HTTPOption) (*http.R
 	if err != nil {
 		return nil, err
 	}
-	httpClient := http.DefaultClient
+	httpClient := forbidRedirectClient
 	if opts != nil && len(opts.Username) != 0 && len(opts.Password) != 0 {
 		req.SetBasicAuth(opts.Username, opts.Password)
 	}
@@ -198,7 +204,7 @@ func HTTPGetResponse(ctx context.Context, url string, opts *HTTPOption) (*http.R
 		}
 		tr.TLSClientConfig = tlsConfig
 		defer tr.CloseIdleConnections()
-		httpClient = &http.Client{Transport: &tr}
+		httpClient = &http.Client{Transport: &tr, CheckRedirect: forbidRedirectFunc}
 	}
 	return httpClient.Do(req)
 }
diff --git a/pkg/utils/common/common_test.go b/pkg/utils/common/common_test.go
index 72805704a..016dc4817 100644
--- a/pkg/utils/common/common_test.go
+++ b/pkg/utils/common/common_test.go
@@ -26,6 +26,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -223,6 +224,25 @@ func TestHttpGetCaFile(t *testing.T) {
 	}
 }
 
+func TestHttpGetForbidRedirect(t *testing.T) {
+	var ctx = context.Background()
+	testServer := &http.Server{Addr: ":19090"}
+
+	http.HandleFunc("/redirect", func(writer http.ResponseWriter, request *http.Request) {
+		http.Redirect(writer, request, "http://192.168.1.1", http.StatusFound)
+	})
+
+	go func() {
+		err := testServer.ListenAndServe()
+		assert.NoError(t, err)
+	}()
+	time.Sleep(time.Millisecond)
+
+	_, err := HTTPGetWithOption(ctx, "http://127.0.0.1:19090/redirect", nil)
+	assert.Error(t, err)
+	assert.True(t, strings.Contains(err.Error(), "got a redirect response which is forbidden"))
+}
+
 func TestGetCUEParameterValue(t *testing.T) {
 	type want struct {
 		err error
diff --git a/pkg/utils/helm/helm_helper.go b/pkg/utils/helm/helm_helper.go
index 78e1a7395..aa155b293 100644
--- a/pkg/utils/helm/helm_helper.go
+++ b/pkg/utils/helm/helm_helper.go
@@ -225,7 +225,7 @@ func (h *Helper) GetIndexInfo(repoURL string, skipCache bool, opts *common.HTTPO
 	}
 	i := &repo.IndexFile{}
 	if err := yaml.UnmarshalStrict(body, i); err != nil {
-		return nil, fmt.Errorf("parse index file from %s failure %w", repoURL, err)
+		return nil, fmt.Errorf("parse index file from %s failure", repoURL)
 	}
 
 	if h.cache != nil {