Commit 147ed42a authored by IanShaw027's avatar IanShaw027
Browse files

fix: restrict payment return urls to internal result page

parent 62ff2d80
...@@ -350,7 +350,7 @@ func (s *PaymentService) invokeProvider(ctx context.Context, order *dbent.Paymen ...@@ -350,7 +350,7 @@ func (s *PaymentService) invokeProvider(ctx context.Context, order *dbent.Paymen
} }
subject := s.buildPaymentSubject(plan, limitAmount, cfg) subject := s.buildPaymentSubject(plan, limitAmount, cfg)
outTradeNo := order.OutTradeNo outTradeNo := order.OutTradeNo
canonicalReturnURL, err := CanonicalizeReturnURL(req.ReturnURL) canonicalReturnURL, err := CanonicalizeReturnURL(req.ReturnURL, req.SrcHost)
if err != nil { if err != nil {
return nil, err return nil, err
} }
......
...@@ -7,6 +7,7 @@ import ( ...@@ -7,6 +7,7 @@ import (
"encoding/base64" "encoding/base64"
"encoding/json" "encoding/json"
"fmt" "fmt"
"net"
"net/url" "net/url"
"strconv" "strconv"
"strings" "strings"
...@@ -16,6 +17,8 @@ import ( ...@@ -16,6 +17,8 @@ import (
infraerrors "github.com/Wei-Shaw/sub2api/internal/pkg/errors" infraerrors "github.com/Wei-Shaw/sub2api/internal/pkg/errors"
) )
const paymentResultReturnPath = "/payment/result"
const ( const (
PaymentSourceHostedRedirect = "hosted_redirect" PaymentSourceHostedRedirect = "hosted_redirect"
PaymentSourceWechatInAppResume = "wechat_in_app_resume" PaymentSourceWechatInAppResume = "wechat_in_app_resume"
...@@ -215,7 +218,7 @@ func visibleMethodSourceSettingKey(method string) string { ...@@ -215,7 +218,7 @@ func visibleMethodSourceSettingKey(method string) string {
} }
} }
func CanonicalizeReturnURL(raw string) (string, error) { func CanonicalizeReturnURL(raw string, srcHost string) (string, error) {
raw = strings.TrimSpace(raw) raw = strings.TrimSpace(raw)
if raw == "" { if raw == "" {
return "", nil return "", nil
...@@ -231,19 +234,29 @@ func CanonicalizeReturnURL(raw string) (string, error) { ...@@ -231,19 +234,29 @@ func CanonicalizeReturnURL(raw string) (string, error) {
if parsed.Path == "" { if parsed.Path == "" {
parsed.Path = "/" parsed.Path = "/"
} }
if parsed.Path != paymentResultReturnPath {
return "", infraerrors.BadRequest("INVALID_RETURN_URL", "return_url must target the canonical internal payment result page")
}
if !sameOriginHost(parsed.Host, srcHost) {
return "", infraerrors.BadRequest("INVALID_RETURN_URL", "return_url must use the same host as the current site")
}
return parsed.String(), nil return parsed.String(), nil
} }
func buildPaymentReturnURL(base string, orderID int64, resumeToken string) (string, error) { func buildPaymentReturnURL(base string, orderID int64, resumeToken string) (string, error) {
canonical, err := CanonicalizeReturnURL(base) canonical := strings.TrimSpace(base)
if err != nil || canonical == "" { if canonical == "" {
return canonical, err return "", nil
} }
parsed, err := url.Parse(canonical) parsed, err := url.Parse(canonical)
if err != nil { if err != nil {
return "", infraerrors.BadRequest("INVALID_RETURN_URL", "return_url must be a valid URL") return "", infraerrors.BadRequest("INVALID_RETURN_URL", "return_url must be a valid URL")
} }
if !parsed.IsAbs() || parsed.Host == "" {
return "", infraerrors.BadRequest("INVALID_RETURN_URL", "return_url must be a valid absolute URL")
}
parsed.Fragment = ""
query := parsed.Query() query := parsed.Query()
if orderID > 0 { if orderID > 0 {
...@@ -258,6 +271,31 @@ func buildPaymentReturnURL(base string, orderID int64, resumeToken string) (stri ...@@ -258,6 +271,31 @@ func buildPaymentReturnURL(base string, orderID int64, resumeToken string) (stri
return parsed.String(), nil return parsed.String(), nil
} }
func sameOriginHost(returnURLHost string, requestHost string) bool {
returnHost := strings.TrimSpace(returnURLHost)
reqHost := strings.TrimSpace(requestHost)
if returnHost == "" || reqHost == "" {
return false
}
if strings.EqualFold(returnHost, reqHost) {
return true
}
returnName, returnPort := splitHostPortDefault(returnHost)
reqName, reqPort := splitHostPortDefault(reqHost)
if returnName == "" || reqName == "" {
return false
}
return strings.EqualFold(returnName, reqName) && returnPort == reqPort
}
func splitHostPortDefault(raw string) (string, string) {
if host, port, err := net.SplitHostPort(raw); err == nil {
return host, port
}
return raw, ""
}
func (s *PaymentResumeService) CreateToken(claims ResumeTokenClaims) (string, error) { func (s *PaymentResumeService) CreateToken(claims ResumeTokenClaims) (string, error) {
if err := s.ensureSigningKey(); err != nil { if err := s.ensureSigningKey(); err != nil {
return "", err return "", err
......
...@@ -64,23 +64,39 @@ func TestNormalizePaymentSource(t *testing.T) { ...@@ -64,23 +64,39 @@ func TestNormalizePaymentSource(t *testing.T) {
func TestCanonicalizeReturnURL(t *testing.T) { func TestCanonicalizeReturnURL(t *testing.T) {
t.Parallel() t.Parallel()
got, err := CanonicalizeReturnURL("https://example.com/pay/result?b=2#a") got, err := CanonicalizeReturnURL("https://example.com/payment/result?b=2#a", "example.com")
if err != nil { if err != nil {
t.Fatalf("CanonicalizeReturnURL returned error: %v", err) t.Fatalf("CanonicalizeReturnURL returned error: %v", err)
} }
if got != "https://example.com/pay/result?b=2" { if got != "https://example.com/payment/result?b=2" {
t.Fatalf("CanonicalizeReturnURL = %q, want %q", got, "https://example.com/pay/result?b=2") t.Fatalf("CanonicalizeReturnURL = %q, want %q", got, "https://example.com/payment/result?b=2")
} }
} }
func TestCanonicalizeReturnURLRejectsRelativeURL(t *testing.T) { func TestCanonicalizeReturnURLRejectsRelativeURL(t *testing.T) {
t.Parallel() t.Parallel()
if _, err := CanonicalizeReturnURL("/payment/result"); err == nil { if _, err := CanonicalizeReturnURL("/payment/result", "example.com"); err == nil {
t.Fatal("CanonicalizeReturnURL should reject relative URLs") t.Fatal("CanonicalizeReturnURL should reject relative URLs")
} }
} }
func TestCanonicalizeReturnURLRejectsExternalHost(t *testing.T) {
t.Parallel()
if _, err := CanonicalizeReturnURL("https://evil.example/payment/result", "app.example.com"); err == nil {
t.Fatal("CanonicalizeReturnURL should reject external hosts")
}
}
func TestCanonicalizeReturnURLRejectsNonCanonicalPath(t *testing.T) {
t.Parallel()
if _, err := CanonicalizeReturnURL("https://app.example.com/orders/42", "app.example.com"); err == nil {
t.Fatal("CanonicalizeReturnURL should reject non-canonical result paths")
}
}
func TestBuildPaymentReturnURL(t *testing.T) { func TestBuildPaymentReturnURL(t *testing.T) {
t.Parallel() t.Parallel()
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment