Skip to content

Commit

Permalink
Fix: forbid 302 request to avoid SSRF (#5000)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
wangyikewxgm committed Nov 4, 2022
1 parent 7f1a901 commit 85489c6
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 4 deletions.
12 changes: 9 additions & 3 deletions pkg/utils/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
20 changes: 20 additions & 0 deletions pkg/utils/common/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/helm/helm_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 85489c6

Please sign in to comment.