From f17774cb3b9612495d89576a91ab3480018cb0b6 Mon Sep 17 00:00:00 2001 From: MHZ Date: Fri, 21 Feb 2025 17:29:17 +0800 Subject: [PATCH] feat: prevent attackers from exploiting redirect attack GetLinkMetadata API (#4428) fix: Prevent attackers from exploiting redirect attack GetLinkMetadata API. --- plugin/httpgetter/html_meta.go | 46 ++++++++++++++++++++++++----- plugin/httpgetter/html_meta_test.go | 20 +++++++++++++ 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/plugin/httpgetter/html_meta.go b/plugin/httpgetter/html_meta.go index 24968d20..50211569 100644 --- a/plugin/httpgetter/html_meta.go +++ b/plugin/httpgetter/html_meta.go @@ -1,16 +1,30 @@ package httpgetter import ( - "errors" "io" "net" "net/http" "net/url" + "github.com/pkg/errors" "golang.org/x/net/html" "golang.org/x/net/html/atom" ) +var ErrInternalIP = errors.New("internal IP addresses are not allowed") + +var httpClient = &http.Client{ + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if err := validateURL(req.URL.String()); err != nil { + return errors.Wrap(err, "redirect to internal IP") + } + if len(via) >= 10 { + return errors.New("too many redirects") + } + return nil + }, +} + type HTMLMeta struct { Title string `json:"title"` Description string `json:"description"` @@ -22,7 +36,7 @@ func GetHTMLMeta(urlStr string) (*HTMLMeta, error) { return nil, err } - response, err := http.Get(urlStr) + response, err := httpClient.Get(urlStr) if err != nil { return nil, err } @@ -110,12 +124,28 @@ func validateURL(urlStr string) error { return errors.New("only http/https protocols are allowed") } - if host := u.Hostname(); host != "" { - ip := net.ParseIP(host) - if ip != nil { - if ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() { - return errors.New("internal IP addresses are not allowed") - } + host := u.Hostname() + if host == "" { + return errors.New("empty hostname") + } + + // check if the hostname is an IP + if ip := net.ParseIP(host); ip != nil { + if ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() { + return errors.Wrap(ErrInternalIP, ip.String()) + } + return nil + } + + // check if it's a hostname, resolve it and check all returned IPs + ips, err := net.LookupIP(host) + if err != nil { + return errors.Errorf("failed to resolve hostname: %v", err) + } + + for _, ip := range ips { + if ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() { + return errors.Wrapf(ErrInternalIP, "host=%s, ip=%s", host, ip.String()) } } diff --git a/plugin/httpgetter/html_meta_test.go b/plugin/httpgetter/html_meta_test.go index 45345d30..d0b0d903 100644 --- a/plugin/httpgetter/html_meta_test.go +++ b/plugin/httpgetter/html_meta_test.go @@ -1,6 +1,8 @@ package httpgetter import ( + "errors" + "strings" "testing" "github.com/stretchr/testify/require" @@ -17,3 +19,21 @@ func TestGetHTMLMeta(t *testing.T) { require.Equal(t, test.htmlMeta, *metadata) } } + +func TestGetHTMLMetaForInternal(t *testing.T) { + // test for internal IP + if _, err := GetHTMLMeta("http://192.168.0.1"); !errors.Is(err, ErrInternalIP) { + t.Errorf("Expected error for internal IP, got %v", err) + } + + // test for resolved internal IP + if _, err := GetHTMLMeta("http://localhost"); !errors.Is(err, ErrInternalIP) { + t.Errorf("Expected error for resolved internal IP, got %v", err) + } + + // test for redirected internal IP + // 49.232.126.226:1110 will redirects to 127.0.0.1 + if _, err := GetHTMLMeta("http://49.232.126.226:1110"); !(errors.Is(err, ErrInternalIP) && strings.Contains(err.Error(), "redirect")) { + t.Errorf("Expected error for redirected internal IP, got %v", err) + } +}