HH-452: cover upload security boundaries (#106)
Co-authored-by: Rogee <rogee@ipao.vip>
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
package security
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"context"
|
||||
"crypto/hmac"
|
||||
"crypto/sha256"
|
||||
@@ -12,6 +13,7 @@ import (
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -614,6 +616,86 @@ func TestSafeRedirectCheckConfig_BlocksPrivateAndNonHTTPRedirects(t *testing.T)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSafeHTTPClient_RevalidatesEveryRealRedirect(t *testing.T) {
|
||||
var firstHits, secondHits, privateHits atomic.Int32
|
||||
private := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
|
||||
privateHits.Add(1)
|
||||
}))
|
||||
defer private.Close()
|
||||
second := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
secondHits.Add(1)
|
||||
http.Redirect(w, r, "http://169.254.169.254/latest/meta-data/", http.StatusFound)
|
||||
}))
|
||||
defer second.Close()
|
||||
first := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
firstHits.Add(1)
|
||||
http.Redirect(w, r, "http://second.example/next", http.StatusFound)
|
||||
}))
|
||||
defer first.Close()
|
||||
|
||||
addresses := map[string]string{
|
||||
"first.example": first.Listener.Addr().String(),
|
||||
"second.example": second.Listener.Addr().String(),
|
||||
"169.254.169.254": private.Listener.Addr().String(),
|
||||
}
|
||||
client := NewSafeHTTPClient(DefaultSSRFConfig())
|
||||
client.client.Transport = &http.Transport{DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
|
||||
host, _, err := net.SplitHostPort(addr)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return (&net.Dialer{}).DialContext(ctx, network, addresses[host])
|
||||
}}
|
||||
|
||||
req, err := http.NewRequest(http.MethodGet, "http://first.example/start", nil)
|
||||
require.NoError(t, err)
|
||||
resp, err := client.Do(req)
|
||||
if resp != nil {
|
||||
resp.Body.Close()
|
||||
}
|
||||
require.ErrorContains(t, err, "private/reserved")
|
||||
assert.Equal(t, int32(1), firstHits.Load())
|
||||
assert.Equal(t, int32(1), secondHits.Load())
|
||||
assert.Zero(t, privateHits.Load())
|
||||
}
|
||||
|
||||
func TestSafeHTTPClient_DialsFirstValidatedDNSResult(t *testing.T) {
|
||||
serverConn, clientConn := net.Pipe()
|
||||
defer serverConn.Close()
|
||||
serverDone := make(chan error, 1)
|
||||
go func() {
|
||||
request, err := http.ReadRequest(bufio.NewReader(serverConn))
|
||||
if err == nil {
|
||||
request.Body.Close()
|
||||
_, err = serverConn.Write([]byte("HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok"))
|
||||
}
|
||||
serverDone <- err
|
||||
}()
|
||||
|
||||
lookups := 0
|
||||
dialedAddress := ""
|
||||
client := newSafeHTTPClient(DefaultSSRFConfig(),
|
||||
func(context.Context, string) ([]net.IPAddr, error) {
|
||||
lookups++
|
||||
if lookups == 1 {
|
||||
return []net.IPAddr{{IP: net.ParseIP("93.184.216.34")}}, nil
|
||||
}
|
||||
return []net.IPAddr{{IP: net.ParseIP("127.0.0.1")}}, nil
|
||||
},
|
||||
func(_ context.Context, _, addr string) (net.Conn, error) {
|
||||
dialedAddress = addr
|
||||
return clientConn, nil
|
||||
},
|
||||
)
|
||||
|
||||
resp, err := client.SafeFetchURL(context.Background(), "http://rebind.example/file")
|
||||
require.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
assert.Equal(t, 1, lookups)
|
||||
assert.Equal(t, "93.184.216.34:80", dialedAddress)
|
||||
require.NoError(t, <-serverDone)
|
||||
}
|
||||
|
||||
func TestValidateURL_Empty(t *testing.T) {
|
||||
err := ValidateURL("", DefaultSSRFConfig())
|
||||
assert.Error(t, err)
|
||||
|
||||
@@ -90,7 +90,10 @@ func NewSafeHTTPClient(cfg SSRFConfig) *SafeHTTPClient {
|
||||
Timeout: 10 * time.Second,
|
||||
KeepAlive: 30 * time.Second,
|
||||
}
|
||||
return newSafeHTTPClient(cfg, net.DefaultResolver.LookupIPAddr, dialer.DialContext)
|
||||
}
|
||||
|
||||
func newSafeHTTPClient(cfg SSRFConfig, lookupIPAddr func(context.Context, string) ([]net.IPAddr, error), dialContext func(context.Context, string, string) (net.Conn, error)) *SafeHTTPClient {
|
||||
transport := &http.Transport{
|
||||
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
|
||||
host, port, err := net.SplitHostPort(addr)
|
||||
@@ -98,7 +101,7 @@ func NewSafeHTTPClient(cfg SSRFConfig) *SafeHTTPClient {
|
||||
return nil, fmt.Errorf("invalid address: %s", addr)
|
||||
}
|
||||
|
||||
ips, err := net.DefaultResolver.LookupIPAddr(ctx, host)
|
||||
ips, err := lookupIPAddr(ctx, host)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("DNS resolution failed for %s: %w", host, err)
|
||||
}
|
||||
@@ -113,7 +116,7 @@ func NewSafeHTTPClient(cfg SSRFConfig) *SafeHTTPClient {
|
||||
// would reopen a DNS-rebinding window between validation and connect.
|
||||
var lastErr error
|
||||
for _, ip := range ips {
|
||||
conn, err := dialer.DialContext(ctx, network, net.JoinHostPort(ip.IP.String(), port))
|
||||
conn, err := dialContext(ctx, network, net.JoinHostPort(ip.IP.String(), port))
|
||||
if err == nil {
|
||||
return conn, nil
|
||||
}
|
||||
|
||||
@@ -1036,6 +1036,11 @@ func readValidatedUpload(reader io.Reader, filename, declaredMIME string, expect
|
||||
return nil, "", fmt.Errorf("unsupported file type: extension %s", filepath.Ext(filename))
|
||||
}
|
||||
}
|
||||
sample := bytes.TrimSpace(data[:min(len(data), 512)])
|
||||
sample = bytes.TrimSpace(bytes.TrimPrefix(sample, []byte{0xef, 0xbb, 0xbf}))
|
||||
if (declaredMIME == "text/plain" || declaredMIME == "text/csv") && bytes.HasPrefix(sample, []byte("<")) {
|
||||
return nil, "", fmt.Errorf("active markup is not allowed for MIME type %s", declaredMIME)
|
||||
}
|
||||
if !uploadMIMEMatches(declaredMIME, detectedMIME) {
|
||||
return nil, "", fmt.Errorf("file content type %s does not match declared type %s", detectedMIME, declaredMIME)
|
||||
}
|
||||
|
||||
@@ -22,6 +22,7 @@ import (
|
||||
"github.com/gochat/gochat/internal/config"
|
||||
"github.com/gochat/gochat/internal/model"
|
||||
"github.com/gochat/gochat/internal/repository"
|
||||
"github.com/gochat/gochat/internal/security"
|
||||
)
|
||||
|
||||
var (
|
||||
@@ -362,6 +363,50 @@ func TestUploadService_RejectsSpoofedMIMEAndSVG(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestUploadService_RejectsTextPlainSVGAndXMLAcrossUploadPaths(t *testing.T) {
|
||||
_, svc, _ := setupUploadServiceTest(t)
|
||||
payloads := map[string][]byte{
|
||||
"svg": []byte(`<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>`),
|
||||
"xml": []byte(`<root><script>alert(1)</script></root>`),
|
||||
}
|
||||
paths := map[string]func(*testing.T, []byte) error{
|
||||
"direct": func(t *testing.T, payload []byte) error {
|
||||
file := createTestFileHeader(t, "attack.txt", payload)
|
||||
file.Header.Set("Content-Type", "text/plain")
|
||||
_, err := svc.AccountDirectUpload(context.Background(), 1, AccountDirectUploadRequest{FileHeader: file})
|
||||
return err
|
||||
},
|
||||
"widget": func(t *testing.T, payload []byte) error {
|
||||
file := createTestFileHeader(t, "attack.txt", payload)
|
||||
file.Header.Set("Content-Type", "text/plain")
|
||||
_, err := svc.WidgetDirectUpload(context.Background(), WidgetDirectUploadRequest{
|
||||
WebsiteToken: "test-website",
|
||||
AuthToken: "test-widget",
|
||||
FileHeader: file,
|
||||
})
|
||||
return err
|
||||
},
|
||||
"remote": func(t *testing.T, payload []byte) error {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Content-Type", "text/plain")
|
||||
_, _ = w.Write(payload)
|
||||
}))
|
||||
defer server.Close()
|
||||
svc.fetchClient = security.NewSafeHTTPClient(security.SSRFConfig{MaxRedirects: 3})
|
||||
_, err := svc.AccountUploadFromURL(context.Background(), 1, server.URL+"/attack.txt")
|
||||
return err
|
||||
},
|
||||
}
|
||||
|
||||
for payloadName, payload := range payloads {
|
||||
for pathName, upload := range paths {
|
||||
t.Run(payloadName+"/"+pathName, func(t *testing.T) {
|
||||
require.ErrorContains(t, upload(t, payload), "active markup")
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestUploadService_AccountUploadFromURLRejectsPrivateTargets(t *testing.T) {
|
||||
_, svc, _ := setupUploadServiceTest(t)
|
||||
for _, target := range []string{
|
||||
|
||||
Reference in New Issue
Block a user