bb73b9a0eea0d902da4811420535842a4f9aae3b
- Author
- Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
- Committer
- GitHub <noreply@github.com>
- Date
Message
Merge commit from fork closes GHSA-vwq2-jx9q-9h9f Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Diff
This diff is truncated to protect this page.
1diff --git a/go.mod b/go.mod
2index 32d153560a1b755d95b46daa41f7f473538c84bc..4f8235f45eba21a0fd0d1b93bf4eeaf70a481a97 100644
3--- a/go.mod
4+++ b/go.mod
5@@ -1,6 +1,6 @@
6 module github.com/charmbracelet/soft-serve
7
8-go 1.23.0
9+go 1.24.0
10
11 require (
12 github.com/alecthomas/chroma/v2 v2.20.0
13diff --git a/pkg/backend/webhooks.go b/pkg/backend/webhooks.go
14index f217c30337f374f7ff0b1aace8c19a86bfb19073..d25d1cf6ec9ddd307d96ba3683ea25ff62882ff6 100644
15--- a/pkg/backend/webhooks.go
16+++ b/pkg/backend/webhooks.go
17@@ -20,6 +20,11 @@ func (b *Backend) CreateWebhook(ctx context.Context, repo proto.Repository, url
18 datastore := store.FromContext(ctx)
19 url = utils.Sanitize(url)
20
21+ // Validate webhook URL to prevent SSRF attacks
22+ if err := webhook.ValidateWebhookURL(url); err != nil {
23+ return err //nolint:wrapcheck
24+ }
25+
26 return dbx.TransactionContext(ctx, func(tx *db.Tx) error {
27 lastID, err := datastore.CreateWebhook(ctx, tx, repo.ID(), url, secret, int(contentType), active)
28 if err != nil {
29@@ -120,6 +125,11 @@ func (b *Backend) UpdateWebhook(ctx context.Context, repo proto.Repository, id i
30 dbx := db.FromContext(ctx)
31 datastore := store.FromContext(ctx)
32
33+ // Validate webhook URL to prevent SSRF attacks
34+ if err := webhook.ValidateWebhookURL(url); err != nil {
35+ return err
36+ }
37+
38 return dbx.TransactionContext(ctx, func(tx *db.Tx) error {
39 if err := datastore.UpdateWebhookByID(ctx, tx, repo.ID(), id, url, secret, int(contentType), active); err != nil {
40 return db.WrapError(err)
41diff --git a/pkg/webhook/ssrf_test.go b/pkg/webhook/ssrf_test.go
42new file mode 100644
43index 0000000000000000000000000000000000000000..0928125b30151996b7b68921027ad4bf1d8fa2ea
44--- /dev/null
45+++ b/pkg/webhook/ssrf_test.go
46@@ -0,0 +1,218 @@
47+package webhook
48+
49+import (
50+ "context"
51+ "net/http"
52+ "net/http/httptest"
53+ "testing"
54+ "time"
55+
56+ "github.com/charmbracelet/soft-serve/pkg/db/models"
57+)
58+
59+// TestSSRFProtection tests that the webhook system blocks SSRF attempts.
60+func TestSSRFProtection(t *testing.T) {
61+ tests := []struct {
62+ name string
63+ webhookURL string
64+ shouldBlock bool
65+ description string
66+ }{
67+ {
68+ name: "block localhost",
69+ webhookURL: "http://localhost:8080/webhook",
70+ shouldBlock: true,
71+ description: "should block localhost addresses",
72+ },
73+ {
74+ name: "block 127.0.0.1",
75+ webhookURL: "http://127.0.0.1:8080/webhook",
76+ shouldBlock: true,
77+ description: "should block loopback addresses",
78+ },
79+ {
80+ name: "block 169.254.169.254",
81+ webhookURL: "http://169.254.169.254/latest/meta-data/",
82+ shouldBlock: true,
83+ description: "should block cloud metadata service",
84+ },
85+ {
86+ name: "block private network",
87+ webhookURL: "http://192.168.1.1/webhook",
88+ shouldBlock: true,
89+ description: "should block private networks",
90+ },
91+ {
92+ name: "allow public IP",
93+ webhookURL: "http://8.8.8.8/webhook",
94+ shouldBlock: false,
95+ description: "should allow public IP addresses",
96+ },
97+ }
98+
99+ for _, tt := range tests {
100+ t.Run(tt.name, func(t *testing.T) {
101+ // Create a test webhook
102+ webhook := models.Webhook{
103+ URL: tt.webhookURL,
104+ ContentType: int(ContentTypeJSON),
105+ Secret: "",
106+ }
107+
108+ // Try to send a webhook
109+ ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
110+ defer cancel()
111+
112+ // Create a simple payload
113+ payload := map[string]string{"test": "data"}
114+
115+ err := sendWebhookWithContext(ctx, webhook, EventPush, payload)
116+
117+ if tt.shouldBlock {
118+ if err == nil {
119+ t.Errorf("%s: expected error but got none", tt.description)
120+ }
121+ } else {
122+ // For public IPs, we expect a connection error (since 8.8.8.8 won't be listening)
123+ // but NOT an SSRF blocking error
124+ if err != nil && isSSRFError(err) {
125+ t.Errorf("%s: should not block public IPs, got: %v", tt.description, err)
126+ }
127+ }
128+ })
129+ }
130+}
131+
132+// TestSecureHTTPClientBlocksRedirects tests that redirects are not followed.
133+func TestSecureHTTPClientBlocksRedirects(t *testing.T) {
134+ // Create a test server on a public-looking address that redirects
135+ redirectServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
136+ http.Redirect(w, r, "http://8.8.8.8:8080/safe", http.StatusFound)
137+ }))
138+ defer redirectServer.Close()
139+
140+ // Try to make a request that would redirect
141+ req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, redirectServer.URL, nil)
142+ if err != nil {
143+ t.Fatalf("Failed to create request: %v", err)
144+ }
145+
146diff --git a/pkg/webhook/validator.go b/pkg/webhook/validator.go
147new file mode 100644
148index 0000000000000000000000000000000000000000..1728a14f5f14293a4fd1c3b947e9659fbda19dd5
149--- /dev/null
150+++ b/pkg/webhook/validator.go
151@@ -0,0 +1,163 @@
152+package webhook
153+
154+import (
155+ "errors"
156+ "fmt"
157+ "net"
158+ "net/url"
159+ "slices"
160+ "strings"
161+)
162+
163+var (
164+ // ErrInvalidScheme is returned when the webhook URL scheme is not http or https.
165+ ErrInvalidScheme = errors.New("webhook URL must use http or https scheme")
166+ // ErrPrivateIP is returned when the webhook URL resolves to a private IP address.
167+ ErrPrivateIP = errors.New("webhook URL cannot resolve to private or internal IP addresses")
168+ // ErrInvalidURL is returned when the webhook URL is invalid.
169+ ErrInvalidURL = errors.New("invalid webhook URL")
170+)
171+
172+// ValidateWebhookURL validates that a webhook URL is safe to use.
173+// It checks:
174+// - URL is properly formatted
175+// - Scheme is http or https
176+// - Hostname does not resolve to private/internal IP addresses
177+// - Hostname is not localhost or similar.
178+func ValidateWebhookURL(rawURL string) error {
179+ if rawURL == "" {
180+ return ErrInvalidURL
181+ }
182+
183+ // Parse the URL
184+ u, err := url.Parse(rawURL)
185+ if err != nil {
186+ return fmt.Errorf("%w: %v", ErrInvalidURL, err)
187+ }
188+
189+ // Check scheme
190+ if u.Scheme != "http" && u.Scheme != "https" {
191+ return ErrInvalidScheme
192+ }
193+
194+ // Extract hostname (without port)
195+ hostname := u.Hostname()
196+ if hostname == "" {
197+ return fmt.Errorf("%w: missing hostname", ErrInvalidURL)
198+ }
199+
200+ // Check for localhost variations
201+ if isLocalhost(hostname) {
202+ return ErrPrivateIP
203+ }
204+
205+ // If it's an IP address, validate it directly
206+ if ip := net.ParseIP(hostname); ip != nil {
207+ if isPrivateOrInternalIP(ip) {
208+ return ErrPrivateIP
209+ }
210+ return nil
211+ }
212+
213+ // Resolve hostname to IP addresses
214+ ips, err := net.LookupIP(hostname)
215+ if err != nil {
216+ return fmt.Errorf("%w: cannot resolve hostname: %v", ErrInvalidURL, err)
217+ }
218+
219+ // Check all resolved IPs
220+ if slices.ContainsFunc(ips, isPrivateOrInternalIP) {
221+ return ErrPrivateIP
222+ }
223+
224+ return nil
225+}
226+
227+// isLocalhost checks if the hostname is localhost or similar.
228+func isLocalhost(hostname string) bool {
229+ hostname = strings.ToLower(hostname)
230+ return hostname == "localhost" ||
231+ hostname == "localhost.localdomain" ||
232+ strings.HasSuffix(hostname, ".localhost")
233+}
234+
235+// isPrivateOrInternalIP checks if an IP address is private, internal, or reserved.
236+func isPrivateOrInternalIP(ip net.IP) bool {
237+ // Loopback addresses (127.0.0.0/8, ::1)
238+ if ip.IsLoopback() {
239+ return true
240+ }
241+
242+ // Link-local addresses (169.254.0.0/16, fe80::/10)
243+ // This blocks AWS/GCP/Azure metadata services
244+ if ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() {
245+ return true
246+ }
247+
248+ // Private addresses (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, fc00::/7)
249+ if ip.IsPrivate() {
250+ return true
251diff --git a/pkg/webhook/validator_test.go b/pkg/webhook/validator_test.go
252new file mode 100644
253index 0000000000000000000000000000000000000000..9d4d4f2ac542dbe3d433e16e18c90d78e6236708
254--- /dev/null
255+++ b/pkg/webhook/validator_test.go
256@@ -0,0 +1,315 @@
257+package webhook
258+
259+import (
260+ "net"
261+ "testing"
262+)
263+
264+func TestValidateWebhookURL(t *testing.T) {
265+ tests := []struct {
266+ name string
267+ url string
268+ wantErr bool
269+ errType error
270+ skip string
271+ }{
272+ // Valid URLs (these will perform DNS lookups, so may fail in some environments)
273+ {
274+ name: "valid https URL",
275+ url: "https://1.1.1.1/webhook",
276+ wantErr: false,
277+ },
278+ {
279+ name: "valid http URL",
280+ url: "http://8.8.8.8/webhook",
281+ wantErr: false,
282+ },
283+ {
284+ name: "valid URL with port",
285+ url: "https://1.1.1.1:8080/webhook",
286+ wantErr: false,
287+ },
288+ {
289+ name: "valid URL with path and query",
290+ url: "https://8.8.8.8/webhook?token=abc123",
291+ wantErr: false,
292+ },
293+
294+ // Invalid schemes
295+ {
296+ name: "ftp scheme",
297+ url: "ftp://example.com/webhook",
298+ wantErr: true,
299+ errType: ErrInvalidScheme,
300+ },
301+ {
302+ name: "file scheme",
303+ url: "file:///etc/passwd",
304+ wantErr: true,
305+ errType: ErrInvalidScheme,
306+ },
307+ {
308+ name: "gopher scheme",
309+ url: "gopher://example.com",
310+ wantErr: true,
311+ errType: ErrInvalidScheme,
312+ },
313+ {
314+ name: "no scheme",
315+ url: "example.com/webhook",
316+ wantErr: true,
317+ errType: ErrInvalidScheme,
318+ },
319+
320+ // Localhost variations
321+ {
322+ name: "localhost",
323+ url: "http://localhost/webhook",
324+ wantErr: true,
325+ errType: ErrPrivateIP,
326+ },
327+ {
328+ name: "localhost with port",
329+ url: "http://localhost:8080/webhook",
330+ wantErr: true,
331+ errType: ErrPrivateIP,
332+ },
333+ {
334+ name: "localhost.localdomain",
335+ url: "http://localhost.localdomain/webhook",
336+ wantErr: true,
337+ errType: ErrPrivateIP,
338+ },
339+
340+ // Loopback IPs
341+ {
342+ name: "127.0.0.1",
343+ url: "http://127.0.0.1/webhook",
344+ wantErr: true,
345+ errType: ErrPrivateIP,
346+ },
347+ {
348+ name: "127.0.0.1 with port",
349+ url: "http://127.0.0.1:8080/webhook",
350+ wantErr: true,
351+ errType: ErrPrivateIP,
352+ },
353+ {
354+ name: "127.1.2.3",
355+ url: "http://127.1.2.3/webhook",
356diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go
357index 15d93bb1db3a16485dad1558f516e4a652ae3bb6..9858c52f4afb313ceb845b8d9133c12bd79965e0 100644
358--- a/pkg/webhook/webhook.go
359+++ b/pkg/webhook/webhook.go
360@@ -10,7 +10,9 @@ import (
361 "errors"
362 "fmt"
363 "io"
364+ "net"
365 "net/http"
366+ "time"
367
368 "github.com/charmbracelet/soft-serve/git"
369 "github.com/charmbracelet/soft-serve/pkg/db"
370@@ -36,6 +38,43 @@ type Delivery struct {
371 Event Event
372 }
373
374+// secureHTTPClient creates an HTTP client with SSRF protection.
375+var secureHTTPClient = &http.Client{
376+ Timeout: 30 * time.Second,
377+ Transport: &http.Transport{
378+ DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
379+ // Parse the address to get the IP
380+ host, _, err := net.SplitHostPort(addr)
381+ if err != nil {
382+ return nil, err //nolint:wrapcheck
383+ }
384+
385+ // Validate the resolved IP before connecting
386+ ip := net.ParseIP(host)
387+ if ip != nil {
388+ if err := ValidateIPBeforeDial(ip); err != nil {
389+ return nil, fmt.Errorf("blocked connection to private IP: %w", err)
390+ }
391+ }
392+
393+ // Use standard dialer with timeout
394+ dialer := &net.Dialer{
395+ Timeout: 10 * time.Second,
396+ KeepAlive: 30 * time.Second,
397+ }
398+ return dialer.DialContext(ctx, network, addr)
399+ },
400+ MaxIdleConns: 100,
401+ IdleConnTimeout: 90 * time.Second,
402+ TLSHandshakeTimeout: 10 * time.Second,
403+ ExpectContinueTimeout: 1 * time.Second,
404+ },
405+ // Don't follow redirects to prevent bypassing IP validation
406+ CheckRedirect: func(*http.Request, []*http.Request) error {
407+ return http.ErrUseLastResponse
408+ },
409+}
410+
411 // do sends a webhook.
412 // Caller must close the returned body.
413 func do(ctx context.Context, url string, method string, headers http.Header, body io.Reader) (*http.Response, error) {
414@@ -45,7 +84,7 @@ func do(ctx context.Context, url string, method string, headers http.Header, bod
415 }
416
417 req.Header = headers
418- res, err := http.DefaultClient.Do(req)
419+ res, err := secureHTTPClient.Do(req)
420 if err != nil {
421 return nil, err
422 }
423diff --git a/testscript/testdata/repo-webhook-ssrf.txtar b/testscript/testdata/repo-webhook-ssrf.txtar
424new file mode 100644
425index 0000000000000000000000000000000000000000..3ae7e441c9ddb1ae291c510168a9787ef2efef07
426--- /dev/null
427+++ b/testscript/testdata/repo-webhook-ssrf.txtar
428@@ -0,0 +1,49 @@
429+# vi: set ft=conf
430+
431+# Test SSRF protection in webhook creation
432+
433+# start soft serve
434+exec soft serve &
435+# wait for SSH server to start
436+ensureserverrunning SSH_PORT
437+
438+# create a repo
439+soft repo create test-repo
440+stderr 'Created repository test-repo.*'
441+
442+# Try to create webhook with localhost - should fail
443+! soft repo webhook create test-repo http://localhost:8080/webhook -e push
444+stderr 'invalid webhook URL.*private'
445+
446+# Try to create webhook with 127.0.0.1 - should fail
447+! soft repo webhook create test-repo http://127.0.0.1:8080/webhook -e push
448+stderr 'invalid webhook URL.*private'
449+
450+# Try to create webhook with AWS metadata service - should fail
451+! soft repo webhook create test-repo http://169.254.169.254/latest/meta-data/ -e push
452+stderr 'invalid webhook URL.*private'
453+
454+# Try to create webhook with private network - should fail
455+! soft repo webhook create test-repo http://192.168.1.1/webhook -e push
456+stderr 'invalid webhook URL.*private'
457+
458+# Try to create webhook with private 10.x network - should fail
459+! soft repo webhook create test-repo http://10.0.0.1/webhook -e push
460+stderr 'invalid webhook URL.*private'
461+
462+# Create webhook with valid public IP - should succeed
463+new-webhook WH_PUBLIC
464+soft repo webhook create test-repo $WH_PUBLIC -e push
465+! stderr 'invalid webhook URL'
466+
467+# List webhooks - should show only the valid one
468+soft repo webhook list test-repo
469+stdout 'webhook.site'
470+
471+# Try to update webhook to localhost - should fail
472+! soft repo webhook update test-repo 1 --url http://localhost:9090/hook
473+stderr 'invalid webhook URL.*private'
474+
475+# stop the server
476+[windows] stopserver
477+[windows] ! stderr .