fix(security): SSRF protection for alarm webhooks (FLAW-05)
Discord and custom webhook URLs are checked when saved and before every call: https only, Discord URLs must point to discord.com. A guarded HTTP client resolves hosts itself and connects only to allowed IPs; localhost, link-local and cloud metadata are always blocked, LAN ranges (incl. Tailscale CGNAT) and plain http only with the new admin switch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
4cdbcbd6b5
commit
62463bc6fd
8 files changed
+288
-16
No files matched your search
+2
-1
@@ -14,7 +14,7 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** (
|
||||
| FLAW-02 | Realtime | ~~Räume ohne Berechtigungsprüfung~~ | **Behoben** |
|
||||
| FLAW-03 | Aufgaben | ~~`ReorderTasks` ändert beliebige Aufgaben per ID (IDOR)~~ | **Behoben** |
|
||||
| FLAW-04 | Auth | ~~Fest eincodiertes Fallback-`JWT_SECRET`~~ | **Behoben** |
|
||||
| FLAW-05 | Benachrichtigungen | Discord-/Webhook-URLs ungeprüft (SSRF) | Mittel |
|
||||
| FLAW-05 | Benachrichtigungen | ~~Discord-/Webhook-URLs ungeprüft (SSRF)~~ | **Behoben** |
|
||||
| FLAW-06 | Aufgaben | ~~Jeder Editor einer Aufgabenliste kann sie per `teamId` an weitere Gruppen freigeben~~ | **Behoben** |
|
||||
| FLAW-07 | Auth | API-Key-`scopes` werden nie ausgewertet; ein Key ist ein Vollzugriff | Mittel |
|
||||
| FLAW-08 | Auth | Webhook-Ingest mit JWT prüft weder Sperrliste noch `token_version` noch `is_active` | Mittel |
|
||||
@@ -66,6 +66,7 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** (
|
||||
- **Maßnahme:** Start verweigern oder Secret beim ersten Start erzeugen und in `/data` ablegen.
|
||||
|
||||
### FLAW-05: SSRF über Alarm-URLs
|
||||
- **Status: Behoben.** [netguard.go](../internal/api/netguard.go): URLs werden beim Speichern und vor jedem Versand geprüft (https Pflicht, Discord nur `discord.com/api/webhooks/`). Der HTTP-Client löst den Host selbst auf und verbindet nur zu erlaubten IPs: localhost, Link-Local/Cloud-Metadaten sind immer gesperrt, Heimnetz-Bereiche (RFC 1918, ULA, CGNAT/Tailscale) und http nur mit dem Admin-Schalter „Alarm-Webhooks ins lokale Netz erlauben“. Keine Umgebungs-Proxys, höchstens 3 Weiterleitungen.
|
||||
- **Fundstellen:** `handleUpdateUserSettings` (keine Validierung), `sendDiscordNotification`, `sendCustomWebhookNotification`.
|
||||
- **Problem:** Jeder Nutzer kann beliebige URLs hinterlegen. Der Server sendet POST-Anfragen dorthin, auch an interne Adressen (`http://169.254.169.254`, Docker-Netz, `localhost`). Über `handleTestUserAlarm` lässt sich das sofort auslösen.
|
||||
- **Maßnahme:** Nur `https`, Discord-Host prüfen, private/Loopback-Adressen beim Verbinden sperren.
|
||||
|
||||
@@ -132,6 +132,7 @@ export const AdminSettingsView: React.FC<AdminSettingsViewProps> = ({
|
||||
smtpFrom: sys.smtpFrom || '',
|
||||
smtpTls: sys.smtpTls ?? true,
|
||||
smtpSkipVerify: Boolean(sys.smtpSkipVerify),
|
||||
allowPrivateWebhooks: Boolean(sys.allowPrivateWebhooks),
|
||||
smtpConfiguredViaEnv: Boolean(sys.smtpConfiguredViaEnv),
|
||||
});
|
||||
}
|
||||
@@ -1081,6 +1082,22 @@ export const AdminSettingsView: React.FC<AdminSettingsViewProps> = ({
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Alarm webhooks into the local network (SSRF protection) */}
|
||||
<div className="p-4 bg-canvas border border-border rounded-card flex items-center justify-between gap-4">
|
||||
<div className="pr-4">
|
||||
<p className="font-bold text-main">Alarm-Webhooks ins lokale Netz erlauben</p>
|
||||
<p className="text-[11px] text-muted mt-0.5">
|
||||
Standard: Discord- und Webhook-Alarme gehen nur an öffentliche https-Adressen. Eingeschaltet sind auch Ziele
|
||||
im Heimnetz erlaubt (z. B. Home Assistant unter 192.168.x.x oder über Tailscale), dort auch per http.
|
||||
Der Server selbst (localhost) und Cloud-Metadaten bleiben immer gesperrt.
|
||||
</p>
|
||||
</div>
|
||||
<ToggleSwitch
|
||||
checked={Boolean(settings.allowPrivateWebhooks)}
|
||||
onChange={(allowPrivateWebhooks) => handleSaveSystemSettings({ allowPrivateWebhooks })}
|
||||
/>
|
||||
</div>
|
||||
|
||||
{/* Public Registration Toggle Switch */}
|
||||
<div className="p-4 bg-canvas border border-border rounded-card flex items-center justify-between gap-4">
|
||||
<div className="pr-4">
|
||||
|
||||
@@ -178,6 +178,7 @@ export interface SystemSettings {
|
||||
smtpFrom?: string;
|
||||
smtpTls?: boolean;
|
||||
smtpSkipVerify?: boolean;
|
||||
allowPrivateWebhooks?: boolean; // alarm webhooks may reach the LAN (and plain http)
|
||||
smtpConfiguredViaEnv?: boolean;
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
// Outgoing alarm requests (Discord, custom webhooks) go to user-supplied URLs. Without checks a user
|
||||
// could make the server call internal services (SSRF): localhost, the Docker network or cloud metadata.
|
||||
|
||||
var errBlockedTarget = errors.New("Ziel nicht erlaubt: interne oder reservierte Adresse")
|
||||
|
||||
func mustCIDRs(list ...string) []*net.IPNet {
|
||||
out := make([]*net.IPNet, 0, len(list))
|
||||
for _, c := range list {
|
||||
_, n, err := net.ParseCIDR(c)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
out = append(out, n)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
var (
|
||||
// Never reachable: this machine, link-local incl. cloud metadata (169.254.169.254), "this network"
|
||||
alwaysBlocked = mustCIDRs("127.0.0.0/8", "::1/128", "169.254.0.0/16", "fe80::/10", "0.0.0.0/8", "::/128")
|
||||
// Home/LAN ranges (incl. CGNAT as used by Tailscale), only with the admin switch
|
||||
// "Ziele im lokalen Netz erlauben"
|
||||
privateRanges = mustCIDRs("10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "fc00::/7", "100.64.0.0/10")
|
||||
)
|
||||
|
||||
func targetIPAllowed(ip net.IP, allowPrivate bool) bool {
|
||||
if ip == nil || ip.IsMulticast() || ip.IsUnspecified() {
|
||||
return false
|
||||
}
|
||||
if v4 := ip.To4(); v4 != nil {
|
||||
ip = v4
|
||||
}
|
||||
for _, n := range alwaysBlocked {
|
||||
if n.Contains(ip) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
if !allowPrivate {
|
||||
for _, n := range privateRanges {
|
||||
if n.Contains(ip) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// validateWebhookURL checks a URL before it is stored or called. HTTPS is required unless the admin
|
||||
// allows local targets (LAN services often speak plain HTTP). Discord URLs must point to Discord.
|
||||
func validateWebhookURL(raw string, discord, allowPrivate bool) error {
|
||||
raw = strings.TrimSpace(raw)
|
||||
if raw == "" {
|
||||
return nil
|
||||
}
|
||||
u, err := url.Parse(raw)
|
||||
if err != nil || u.Host == "" {
|
||||
return errors.New("Ungültige URL")
|
||||
}
|
||||
switch u.Scheme {
|
||||
case "https":
|
||||
case "http":
|
||||
if !allowPrivate {
|
||||
return errors.New("Nur https-URLs sind erlaubt (http nur mit freigegebenen Zielen im lokalen Netz)")
|
||||
}
|
||||
default:
|
||||
return errors.New("Nur https-URLs sind erlaubt")
|
||||
}
|
||||
if discord {
|
||||
host := strings.ToLower(u.Hostname())
|
||||
if u.Scheme != "https" || (host != "discord.com" && host != "discordapp.com") || !strings.HasPrefix(u.Path, "/api/webhooks/") {
|
||||
return errors.New("Discord-Webhooks müssen mit https://discord.com/api/webhooks/ beginnen")
|
||||
}
|
||||
}
|
||||
if ip := net.ParseIP(u.Hostname()); ip != nil && !targetIPAllowed(ip, allowPrivate) {
|
||||
return errBlockedTarget
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// guardedClient resolves the host itself and only connects to allowed addresses. Checking at dial time
|
||||
// (not just the URL) also covers DNS tricks and redirects; proxies from the environment are not used.
|
||||
func guardedClient(allowPrivate bool) *http.Client {
|
||||
dialer := &net.Dialer{Timeout: 5 * time.Second}
|
||||
transport := &http.Transport{
|
||||
Proxy: nil,
|
||||
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
|
||||
host, port, err := net.SplitHostPort(addr)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
ips, err := net.DefaultResolver.LookupIPAddr(ctx, host)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
for _, ip := range ips {
|
||||
if targetIPAllowed(ip.IP, allowPrivate) {
|
||||
return dialer.DialContext(ctx, network, net.JoinHostPort(ip.IP.String(), port))
|
||||
}
|
||||
}
|
||||
return nil, fmt.Errorf("%w (%s)", errBlockedTarget, host)
|
||||
},
|
||||
TLSHandshakeTimeout: 5 * time.Second,
|
||||
ResponseHeaderTimeout: 10 * time.Second,
|
||||
MaxIdleConns: 10,
|
||||
}
|
||||
return &http.Client{
|
||||
Timeout: 10 * time.Second,
|
||||
Transport: transport,
|
||||
CheckRedirect: func(req *http.Request, via []*http.Request) error {
|
||||
if len(via) >= 3 {
|
||||
return errors.New("zu viele Weiterleitungen")
|
||||
}
|
||||
return nil
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func (s *Server) allowPrivateWebhooks() bool {
|
||||
sys, _ := s.store.GetSystemSettings()
|
||||
return sys != nil && sys.AllowPrivateWebhooks
|
||||
}
|
||||
@@ -0,0 +1,79 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestTargetIPAllowed(t *testing.T) {
|
||||
cases := []struct {
|
||||
ip string
|
||||
allowPrivate bool
|
||||
want bool
|
||||
}{
|
||||
{"127.0.0.1", true, false},
|
||||
{"::1", true, false},
|
||||
{"169.254.169.254", true, false}, // cloud metadata
|
||||
{"0.0.0.0", true, false},
|
||||
{"192.168.1.20", false, false},
|
||||
{"192.168.1.20", true, true},
|
||||
{"10.0.0.5", false, false},
|
||||
{"100.101.102.103", true, true}, // Tailscale
|
||||
{"fd00::1", false, false},
|
||||
{"93.184.216.34", false, true},
|
||||
{"::ffff:127.0.0.1", true, false}, // IPv4-mapped loopback
|
||||
}
|
||||
for _, c := range cases {
|
||||
if got := targetIPAllowed(net.ParseIP(c.ip), c.allowPrivate); got != c.want {
|
||||
t.Errorf("targetIPAllowed(%s, private=%v) = %v, want %v", c.ip, c.allowPrivate, got, c.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateWebhookURL(t *testing.T) {
|
||||
ok := []struct {
|
||||
url string
|
||||
discord, private bool
|
||||
}{
|
||||
{"https://discord.com/api/webhooks/1/abc", true, false},
|
||||
{"https://hooks.example.org/x", false, false},
|
||||
{"http://192.168.1.20:8123/api/webhook/x", false, true},
|
||||
{"", false, false},
|
||||
}
|
||||
for _, c := range ok {
|
||||
if err := validateWebhookURL(c.url, c.discord, c.private); err != nil {
|
||||
t.Errorf("%q should be allowed: %v", c.url, err)
|
||||
}
|
||||
}
|
||||
bad := []struct {
|
||||
url string
|
||||
discord, private bool
|
||||
}{
|
||||
{"http://hooks.example.org/x", false, false},
|
||||
{"https://evil.example.org/api/webhooks/1", true, false},
|
||||
{"https://127.0.0.1/x", false, true},
|
||||
{"https://169.254.169.254/latest/meta-data", false, true},
|
||||
{"https://192.168.1.20/x", false, false},
|
||||
{"file:///etc/passwd", false, true},
|
||||
{"gopher://x", false, false},
|
||||
}
|
||||
for _, c := range bad {
|
||||
if err := validateWebhookURL(c.url, c.discord, c.private); err == nil {
|
||||
t.Errorf("%q must be rejected", c.url)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A hostname that resolves to localhost (like a test server or a DNS rebinding trick) is refused at
|
||||
// connection time even though the URL itself looks harmless.
|
||||
func TestGuardedClientRefusesLoopbackAtDial(t *testing.T) {
|
||||
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(204) }))
|
||||
defer ts.Close()
|
||||
url := strings.Replace(ts.URL, "127.0.0.1", "localhost", 1)
|
||||
if _, err := guardedClient(true).Get(url); err == nil || !strings.Contains(err.Error(), "nicht erlaubt") {
|
||||
t.Fatalf("loopback must be refused at dial time, got %v", err)
|
||||
}
|
||||
}
|
||||
+30
-2
@@ -2325,6 +2325,20 @@ func (s *Server) handleUpdateUserSettings(w http.ResponseWriter, r *http.Request
|
||||
writeError(w, http.StatusBadRequest, "Ungültiges JSON")
|
||||
return
|
||||
}
|
||||
// Alarm URLs are checked before they are stored (SSRF protection, see netguard.go)
|
||||
allowPrivate := s.allowPrivateWebhooks()
|
||||
if req.DiscordWebhookURL != nil {
|
||||
if err := validateWebhookURL(*req.DiscordWebhookURL, true, allowPrivate); err != nil {
|
||||
writeError(w, http.StatusBadRequest, "Discord-Webhook: "+err.Error())
|
||||
return
|
||||
}
|
||||
}
|
||||
if req.CustomWebhookURL != nil {
|
||||
if err := validateWebhookURL(*req.CustomWebhookURL, false, allowPrivate); err != nil {
|
||||
writeError(w, http.StatusBadRequest, "Webhook-URL: "+err.Error())
|
||||
return
|
||||
}
|
||||
}
|
||||
if req.Password != nil && strings.TrimSpace(*req.Password) != "" {
|
||||
hash, err := auth.HashPassword(*req.Password)
|
||||
if err != nil {
|
||||
@@ -2933,6 +2947,12 @@ func (s *Server) handleAdminUpdateUser(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
if req.DiscordWebhookURL != nil {
|
||||
if err := validateWebhookURL(*req.DiscordWebhookURL, true, s.allowPrivateWebhooks()); err != nil {
|
||||
writeError(w, http.StatusBadRequest, "Discord-Webhook: "+err.Error())
|
||||
return
|
||||
}
|
||||
}
|
||||
u, err := s.store.UpdateUser(targetUserID, req.DisplayName, req.Email, req.Role, req.IsActive, req.DiscordWebhookURL, nil)
|
||||
if err != nil {
|
||||
writeError(w, http.StatusBadRequest, err.Error())
|
||||
@@ -3112,7 +3132,11 @@ func (s *Server) sendEmailNotification(toEmail, subject, bodyText string) error
|
||||
}
|
||||
|
||||
func (s *Server) sendDiscordNotification(webhookURL, title, description, dueDate, priority string) error {
|
||||
httpClient := &http.Client{Timeout: 10 * time.Second}
|
||||
allowPrivate := s.allowPrivateWebhooks()
|
||||
if err := validateWebhookURL(webhookURL, true, allowPrivate); err != nil {
|
||||
return err
|
||||
}
|
||||
httpClient := guardedClient(allowPrivate)
|
||||
payload := map[string]interface{}{
|
||||
"content": "⏰ **MindSift Aufgaben-Alarm**",
|
||||
"embeds": []map[string]interface{}{
|
||||
@@ -3142,7 +3166,11 @@ func (s *Server) sendDiscordNotification(webhookURL, title, description, dueDate
|
||||
}
|
||||
|
||||
func (s *Server) sendCustomWebhookNotification(webhookURL, event, title, notes, dueDate, priority string) error {
|
||||
httpClient := &http.Client{Timeout: 10 * time.Second}
|
||||
allowPrivate := s.allowPrivateWebhooks()
|
||||
if err := validateWebhookURL(webhookURL, false, allowPrivate); err != nil {
|
||||
return err
|
||||
}
|
||||
httpClient := guardedClient(allowPrivate)
|
||||
payload := map[string]interface{}{
|
||||
"event": event,
|
||||
"title": title,
|
||||
|
||||
+14
-11
@@ -278,17 +278,20 @@ type TeamMember struct {
|
||||
}
|
||||
|
||||
type SystemSettings struct {
|
||||
SharingMode string `json:"sharingMode"` // private | organizations | open
|
||||
AllowRegistration bool `json:"allowRegistration"`
|
||||
InstanceName string `json:"instanceName"`
|
||||
SMTPHost string `json:"smtpHost"`
|
||||
SMTPPort string `json:"smtpPort"`
|
||||
SMTPUser string `json:"smtpUser"`
|
||||
SMTPPass string `json:"smtpPass"`
|
||||
SMTPFrom string `json:"smtpFrom"`
|
||||
SMTPTLS bool `json:"smtpTls"`
|
||||
SMTPSkipVerify bool `json:"smtpSkipVerify"` // accept self-signed certificates
|
||||
SMTPConfiguredViaEnv bool `json:"smtpConfiguredViaEnv,omitempty"`
|
||||
SharingMode string `json:"sharingMode"` // private | organizations | open
|
||||
AllowRegistration bool `json:"allowRegistration"`
|
||||
InstanceName string `json:"instanceName"`
|
||||
SMTPHost string `json:"smtpHost"`
|
||||
SMTPPort string `json:"smtpPort"`
|
||||
SMTPUser string `json:"smtpUser"`
|
||||
SMTPPass string `json:"smtpPass"`
|
||||
SMTPFrom string `json:"smtpFrom"`
|
||||
SMTPTLS bool `json:"smtpTls"`
|
||||
SMTPSkipVerify bool `json:"smtpSkipVerify"` // accept self-signed certificates
|
||||
// AllowPrivateWebhooks lets alarm webhooks reach LAN addresses (and plain http); localhost and cloud
|
||||
// metadata stay blocked
|
||||
AllowPrivateWebhooks bool `json:"allowPrivateWebhooks"`
|
||||
SMTPConfiguredViaEnv bool `json:"smtpConfiguredViaEnv,omitempty"`
|
||||
}
|
||||
|
||||
// UnmarshalJSON accepts smtpPort as text or number: older clients sent a number, which made the whole
|
||||
|
||||
@@ -490,7 +490,8 @@ func (s *Store) migrate() error {
|
||||
('smtp_pass', ''),
|
||||
('smtp_from', ''),
|
||||
('smtp_tls', 'true'),
|
||||
('smtp_skip_verify', 'false')`)
|
||||
('smtp_skip_verify', 'false'),
|
||||
('allow_private_webhooks', 'false')`)
|
||||
|
||||
// Clean up legacy roles in team_members and users
|
||||
_, _ = s.db.Exec(`UPDATE team_members SET role = 'admin' WHERE role IN ('team_admin', 'owner')`)
|
||||
@@ -5090,6 +5091,8 @@ func (s *Store) GetSystemSettings() (*domain.SystemSettings, error) {
|
||||
settings.SMTPTLS = (v != "false" && v != "0")
|
||||
case "smtp_skip_verify":
|
||||
settings.SMTPSkipVerify = v == "true"
|
||||
case "allow_private_webhooks":
|
||||
settings.AllowPrivateWebhooks = v == "true"
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -5109,6 +5112,10 @@ func (s *Store) UpdateSystemSettings(settings *domain.SystemSettings) error {
|
||||
if settings.SMTPSkipVerify {
|
||||
skipVerifyStr = "true"
|
||||
}
|
||||
privateHooksStr := "false"
|
||||
if settings.AllowPrivateWebhooks {
|
||||
privateHooksStr = "true"
|
||||
}
|
||||
if settings.SMTPPort == "" {
|
||||
settings.SMTPPort = "587"
|
||||
}
|
||||
@@ -5126,7 +5133,8 @@ func (s *Store) UpdateSystemSettings(settings *domain.SystemSettings) error {
|
||||
('smtp_pass', ?),
|
||||
('smtp_from', ?),
|
||||
('smtp_tls', ?),
|
||||
('smtp_skip_verify', ?)
|
||||
('smtp_skip_verify', ?),
|
||||
('allow_private_webhooks', ?)
|
||||
ON CONFLICT(key) DO UPDATE SET value = excluded.value`,
|
||||
settings.SharingMode,
|
||||
regStr,
|
||||
@@ -5138,6 +5146,7 @@ func (s *Store) UpdateSystemSettings(settings *domain.SystemSettings) error {
|
||||
strings.TrimSpace(settings.SMTPFrom),
|
||||
tlsStr,
|
||||
skipVerifyStr,
|
||||
privateHooksStr,
|
||||
)
|
||||
return err
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user