diff --git a/Dockerfile b/Dockerfile index 7bdb71d5..311f87ef 100644 --- a/Dockerfile +++ b/Dockerfile @@ -199,7 +199,8 @@ RUN --mount=type=cache,target=/root/.cache/go-build \ # ---- CrowdSec Builder ---- # Build CrowdSec from source to ensure we use Go 1.25.5+ and avoid stdlib vulnerabilities # (CVE-2025-58183, CVE-2025-58186, CVE-2025-58187, CVE-2025-61729) -FROM --platform=$BUILDPLATFORM golang:1.25-alpine AS crowdsec-builder +# renovate: datasource=docker depName=golang versioning=docker +FROM --platform=$BUILDPLATFORM golang:1.25.5-alpine AS crowdsec-builder COPY --from=xx / / WORKDIR /tmp/crowdsec diff --git a/backend/internal/services/mail_service.go b/backend/internal/services/mail_service.go index f81bc394..c885788a 100644 --- a/backend/internal/services/mail_service.go +++ b/backend/internal/services/mail_service.go @@ -8,7 +8,7 @@ import ( "html/template" "net/mail" "net/smtp" - "regexp" + "net/url" "strings" "github.com/Wikid82/charon/backend/internal/logger" @@ -16,9 +16,54 @@ import ( "gorm.io/gorm" ) -// emailHeaderSanitizer removes CR, LF, and other control characters that could -// enable header injection attacks (CWE-93: Improper Neutralization of CRLF). -var emailHeaderSanitizer = regexp.MustCompile(`[\x00-\x1f\x7f]`) +var errEmailHeaderInjection = errors.New("email header value contains CR/LF") + +var errInvalidBaseURLForInvite = errors.New("baseURL must start with http:// or https:// and cannot include path components") + +type emailHeaderName string + +const ( + headerFrom emailHeaderName = "From" + headerTo emailHeaderName = "To" + headerReplyTo emailHeaderName = "Reply-To" + headerSubject emailHeaderName = "Subject" +) + +func rejectCRLF(value string) error { + if strings.ContainsAny(value, "\r\n") { + return errEmailHeaderInjection + } + return nil +} + +func normalizeBaseURLForInvite(raw string) (string, error) { + if raw == "" { + return "", errInvalidBaseURLForInvite + } + if err := rejectCRLF(raw); err != nil { + return "", errInvalidBaseURLForInvite + } + + parsed, err := url.Parse(raw) + if err != nil { + return "", errInvalidBaseURLForInvite + } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return "", errInvalidBaseURLForInvite + } + if parsed.Host == "" { + return "", errInvalidBaseURLForInvite + } + if parsed.Path != "" && parsed.Path != "/" { + return "", errInvalidBaseURLForInvite + } + if parsed.RawQuery != "" || parsed.Fragment != "" || parsed.User != nil { + return "", errInvalidBaseURLForInvite + } + + // Rebuild from parsed, validated components so we don't propagate any other parts. + return (&url.URL{Scheme: parsed.Scheme, Host: parsed.Host}).String(), nil +} // SMTPConfig holds the SMTP server configuration. type SMTPConfig struct { @@ -196,16 +241,34 @@ func (s *MailService) SendEmail(to, subject, htmlBody string) error { return errors.New("SMTP not configured") } - // Validate email addresses to prevent injection attacks - if err := validateEmailAddress(to); err != nil { + if strings.ContainsAny(subject, "\r\n") { + return fmt.Errorf("invalid subject: %w", errEmailHeaderInjection) + } + + toAddr, err := parseEmailAddressForHeader(headerTo, to) + if err != nil { return fmt.Errorf("invalid recipient address: %w", err) } - if err := validateEmailAddress(config.FromAddress); err != nil { + + fromAddr, err := parseEmailAddressForHeader(headerFrom, config.FromAddress) + if err != nil { return fmt.Errorf("invalid from address: %w", err) } - // Build the email message (headers are sanitized in buildEmail) - msg := s.buildEmail(config.FromAddress, to, subject, htmlBody) + // Build the email message (headers are validated and formatted) + msg, err := s.buildEmail(fromAddr, toAddr, nil, subject, htmlBody) + if err != nil { + return err + } + + fromEnvelope := fromAddr.Address + toEnvelope := toAddr.Address + if err := rejectCRLF(fromEnvelope); err != nil { + return fmt.Errorf("invalid from address: %w", err) + } + if err := rejectCRLF(toEnvelope); err != nil { + return fmt.Errorf("invalid recipient address: %w", err) + } addr := fmt.Sprintf("%s:%d", config.Host, config.Port) var auth smtp.Auth @@ -215,51 +278,119 @@ func (s *MailService) SendEmail(to, subject, htmlBody string) error { switch config.Encryption { case "ssl": - return s.sendSSL(addr, config, auth, to, msg) + return s.sendSSL(addr, config, auth, fromEnvelope, toEnvelope, msg) case "starttls": - return s.sendSTARTTLS(addr, config, auth, to, msg) + return s.sendSTARTTLS(addr, config, auth, fromEnvelope, toEnvelope, msg) default: - return smtp.SendMail(addr, auth, config.FromAddress, []string{to}, msg) + return smtp.SendMail(addr, auth, fromEnvelope, []string{toEnvelope}, msg) } } -// buildEmail constructs a properly formatted email message with sanitized headers. -// All header values are sanitized to prevent email header injection (CWE-93). +// buildEmail constructs a properly formatted email message with validated headers. // -// Security Note: Email injection protection implemented via: -// - Headers sanitized by sanitizeEmailHeader() removing control chars (0x00-0x1F, 0x7F) -// - Body protected by sanitizeEmailBody() with RFC 5321 dot-stuffing -// - mail.FormatAddress validates RFC 5322 address format -// CodeQL taint tracking warning intentionally kept as architectural guardrail -func (s *MailService) buildEmail(from, to, subject, htmlBody string) []byte { - // Sanitize all header values to prevent CRLF injection - sanitizedFrom := sanitizeEmailHeader(from) - sanitizedTo := sanitizeEmailHeader(to) - sanitizedSubject := sanitizeEmailHeader(subject) +// Security note: +// - Rejects CR/LF in header values to prevent email header injection (CWE-93). +// - Uses net/mail parsing/formatting for address headers. +// - Body protected by sanitizeEmailBody() with RFC 5321 dot-stuffing. +func (s *MailService) buildEmail(fromAddr, toAddr, replyToAddr *mail.Address, subject, htmlBody string) ([]byte, error) { + if fromAddr == nil { + return nil, errors.New("from address is required") + } + if toAddr == nil { + return nil, errors.New("to address is required") + } + if strings.ContainsAny(subject, "\r\n") { + return nil, fmt.Errorf("invalid subject: %w", errEmailHeaderInjection) + } - headers := make(map[string]string) - headers["From"] = sanitizedFrom - headers["To"] = sanitizedTo - headers["Subject"] = sanitizedSubject - headers["MIME-Version"] = "1.0" - headers["Content-Type"] = "text/html; charset=UTF-8" + fromHeader, err := formatEmailAddressForHeader(headerFrom, fromAddr) + if err != nil { + return nil, fmt.Errorf("invalid from address: %w", err) + } + toHeader, err := formatEmailAddressForHeader(headerTo, toAddr) + if err != nil { + return nil, fmt.Errorf("invalid recipient address: %w", err) + } + + var replyToHeader string + if replyToAddr != nil { + replyToHeader, err = formatEmailAddressForHeader(headerReplyTo, replyToAddr) + if err != nil { + return nil, fmt.Errorf("invalid reply-to address: %w", err) + } + } var msg bytes.Buffer - for key, value := range headers { - msg.WriteString(fmt.Sprintf("%s: %s\r\n", key, value)) + if err := writeEmailHeader(&msg, headerFrom, fromHeader); err != nil { + return nil, err } + if err := writeEmailHeader(&msg, headerTo, toHeader); err != nil { + return nil, err + } + if replyToHeader != "" { + if err := writeEmailHeader(&msg, headerReplyTo, replyToHeader); err != nil { + return nil, err + } + } + if err := writeEmailHeader(&msg, headerSubject, subject); err != nil { + return nil, err + } + msg.WriteString("MIME-Version: 1.0\r\n") + msg.WriteString("Content-Type: text/html; charset=UTF-8\r\n") msg.WriteString("\r\n") - // Sanitize body to prevent SMTP injection (CWE-93) + sanitizedBody := sanitizeEmailBody(htmlBody) msg.WriteString(sanitizedBody) - return msg.Bytes() + return msg.Bytes(), nil } -// sanitizeEmailHeader removes CR, LF, and control characters from email header -// values to prevent email header injection attacks (CWE-93). -func sanitizeEmailHeader(value string) string { - return emailHeaderSanitizer.ReplaceAllString(value, "") +func rejectEmailHeaderValueCRLF(_ emailHeaderName, value string) error { + return rejectCRLF(value) +} + +func parseEmailAddressForHeader(field emailHeaderName, raw string) (*mail.Address, error) { + if raw == "" { + return nil, errors.New("email address is empty") + } + if strings.ContainsAny(raw, "\r\n") { + return nil, errEmailHeaderInjection + } + addr, err := mail.ParseAddress(raw) + if err != nil { + return nil, fmt.Errorf("invalid email address: %w", err) + } + if strings.ContainsAny(addr.String(), "\r\n") { + return nil, errEmailHeaderInjection + } + return addr, nil +} + +func formatEmailAddressForHeader(field emailHeaderName, addr *mail.Address) (string, error) { + if addr == nil { + return "", errors.New("email address is nil") + } + // Check the name field directly before encoding (CodeQL go/email-injection) + // net/mail.Address.String() MIME-encodes special chars, but we reject them upfront + if strings.ContainsAny(addr.Name, "\r\n") { + return "", errEmailHeaderInjection + } + formatted := addr.String() + if strings.ContainsAny(formatted, "\r\n") { + return "", errEmailHeaderInjection + } + return formatted, nil +} + +func writeEmailHeader(buf *bytes.Buffer, header emailHeaderName, value string) error { + if strings.ContainsAny(value, "\r\n") { + return fmt.Errorf("invalid %s header: %w", header, errEmailHeaderInjection) + } + buf.WriteString(string(header)) + buf.WriteString(": ") + buf.WriteString(value) + buf.WriteString("\r\n") + return nil } // sanitizeEmailBody performs SMTP dot-stuffing to prevent email injection. @@ -276,21 +407,8 @@ func sanitizeEmailBody(body string) string { return strings.Join(lines, "\n") } -// validateEmailAddress validates that an email address is well-formed. -// Returns an error if the address is invalid. -func validateEmailAddress(email string) error { - if email == "" { - return errors.New("email address is empty") - } - _, err := mail.ParseAddress(email) - if err != nil { - return fmt.Errorf("invalid email address: %w", err) - } - return nil -} - // sendSSL sends email using direct SSL/TLS connection. -func (s *MailService) sendSSL(addr string, config *SMTPConfig, auth smtp.Auth, to string, msg []byte) error { +func (s *MailService) sendSSL(addr string, config *SMTPConfig, auth smtp.Auth, fromEnvelope, toEnvelope string, msg []byte) error { tlsConfig := &tls.Config{ ServerName: config.Host, MinVersion: tls.VersionTLS12, @@ -322,11 +440,11 @@ func (s *MailService) sendSSL(addr string, config *SMTPConfig, auth smtp.Auth, t } } - if err := client.Mail(config.FromAddress); err != nil { + if err := client.Mail(fromEnvelope); err != nil { return fmt.Errorf("MAIL FROM failed: %w", err) } - if err := client.Rcpt(to); err != nil { + if err := client.Rcpt(toEnvelope); err != nil { return fmt.Errorf("RCPT TO failed: %w", err) } @@ -349,7 +467,7 @@ func (s *MailService) sendSSL(addr string, config *SMTPConfig, auth smtp.Auth, t } // sendSTARTTLS sends email using STARTTLS. -func (s *MailService) sendSTARTTLS(addr string, config *SMTPConfig, auth smtp.Auth, to string, msg []byte) error { +func (s *MailService) sendSTARTTLS(addr string, config *SMTPConfig, auth smtp.Auth, fromEnvelope, toEnvelope string, msg []byte) error { client, err := smtp.Dial(addr) if err != nil { return fmt.Errorf("SMTP connection failed: %w", err) @@ -375,11 +493,11 @@ func (s *MailService) sendSTARTTLS(addr string, config *SMTPConfig, auth smtp.Au } } - if err := client.Mail(config.FromAddress); err != nil { + if err := client.Mail(fromEnvelope); err != nil { return fmt.Errorf("MAIL FROM failed: %w", err) } - if err := client.Rcpt(to); err != nil { + if err := client.Rcpt(toEnvelope); err != nil { return fmt.Errorf("RCPT TO failed: %w", err) } @@ -403,21 +521,30 @@ func (s *MailService) sendSTARTTLS(addr string, config *SMTPConfig, auth smtp.Au // SendInvite sends an invitation email to a new user. func (s *MailService) SendInvite(email, inviteToken, appName, baseURL string) error { - // Validate inputs to prevent content spoofing (CWE-93) - if err := validateEmailAddress(email); err != nil { + if _, err := parseEmailAddressForHeader(headerTo, email); err != nil { return fmt.Errorf("invalid email address: %w", err) } - // Sanitize appName to prevent injection in email content - appName = sanitizeEmailHeader(strings.TrimSpace(appName)) + + appName = strings.TrimSpace(appName) if appName == "" { appName = "Application" } - // Validate baseURL format + // Validate appName to prevent CRLF injection in subject line (CodeQL go/email-injection) + if err := rejectCRLF(appName); err != nil { + return fmt.Errorf("invalid app name: %w", err) + } + baseURL = strings.TrimSpace(baseURL) if baseURL == "" { return errors.New("baseURL cannot be empty") } + normalizedBaseURL, err := normalizeBaseURLForInvite(baseURL) + if err != nil { + return err + } + baseURL = normalizedBaseURL + inviteURL := fmt.Sprintf("%s/accept-invite?token=%s", strings.TrimSuffix(baseURL, "/"), inviteToken) tmpl := ` diff --git a/backend/internal/services/mail_service_test.go b/backend/internal/services/mail_service_test.go index cac1e15c..a7d1119d 100644 --- a/backend/internal/services/mail_service_test.go +++ b/backend/internal/services/mail_service_test.go @@ -1,6 +1,7 @@ package services import ( + "net/mail" "strings" "testing" @@ -37,11 +38,9 @@ func TestMailService_SaveAndGetSMTPConfig(t *testing.T) { Encryption: "starttls", } - // Save config err := svc.SaveSMTPConfig(config) require.NoError(t, err) - // Retrieve config retrieved, err := svc.GetSMTPConfig() require.NoError(t, err) @@ -57,7 +56,6 @@ func TestMailService_UpdateSMTPConfig(t *testing.T) { db := setupMailTestDB(t) svc := NewMailService(db) - // Save initial config config := &SMTPConfig{ Host: "smtp.example.com", Port: 587, @@ -69,14 +67,12 @@ func TestMailService_UpdateSMTPConfig(t *testing.T) { err := svc.SaveSMTPConfig(config) require.NoError(t, err) - // Update config config.Host = "smtp.newhost.com" config.Port = 465 config.Encryption = "ssl" err = svc.SaveSMTPConfig(config) require.NoError(t, err) - // Verify update retrieved, err := svc.GetSMTPConfig() require.NoError(t, err) @@ -137,11 +133,9 @@ func TestMailService_GetSMTPConfig_Defaults(t *testing.T) { db := setupMailTestDB(t) svc := NewMailService(db) - // Get config without saving anything config, err := svc.GetSMTPConfig() require.NoError(t, err) - // Should have defaults assert.Equal(t, 587, config.Port) assert.Equal(t, "starttls", config.Encryption) assert.Empty(t, config.Host) @@ -151,110 +145,26 @@ func TestMailService_BuildEmail(t *testing.T) { db := setupMailTestDB(t) svc := NewMailService(db) - msg := svc.buildEmail( - "sender@example.com", - "recipient@example.com", - "Test Subject", - "Test Body", - ) + fromAddr, err := mail.ParseAddress("sender@example.com") + require.NoError(t, err) + toAddr, err := mail.ParseAddress("recipient@example.com") + require.NoError(t, err) + + msg, err := svc.buildEmail(fromAddr, toAddr, nil, "Test Subject", "Test Body") + require.NoError(t, err) msgStr := string(msg) - assert.Contains(t, msgStr, "From: sender@example.com") - assert.Contains(t, msgStr, "To: recipient@example.com") + // Addresses are RFC-formatted and may include angle brackets + assert.Contains(t, msgStr, "From:") + assert.Contains(t, msgStr, "sender@example.com") + assert.Contains(t, msgStr, "To:") + assert.Contains(t, msgStr, "recipient@example.com") assert.Contains(t, msgStr, "Subject: Test Subject") assert.Contains(t, msgStr, "Content-Type: text/html") assert.Contains(t, msgStr, "Test Body") } -// TestMailService_HeaderInjectionPrevention tests that CRLF injection is prevented (CWE-93) -func TestMailService_HeaderInjectionPrevention(t *testing.T) { - db := setupMailTestDB(t) - svc := NewMailService(db) - - tests := []struct { - name string - subject string - subjectShouldBe string // The sanitized subject line - }{ - { - name: "subject with CRLF injection attempt", - subject: "Normal Subject\r\nBcc: attacker@evil.com", - subjectShouldBe: "Normal SubjectBcc: attacker@evil.com", // CRLF stripped, text concatenated - }, - { - name: "subject with LF injection attempt", - subject: "Normal Subject\nX-Injected: malicious", - subjectShouldBe: "Normal SubjectX-Injected: malicious", - }, - { - name: "subject with null byte", - subject: "Normal Subject\x00Hidden", - subjectShouldBe: "Normal SubjectHidden", - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - msg := svc.buildEmail( - "sender@example.com", - "recipient@example.com", - tc.subject, - "

Body

", - ) - - msgStr := string(msg) - - // Verify sanitized subject appears - assert.Contains(t, msgStr, "Subject: "+tc.subjectShouldBe) - - // Split by the header/body separator to get headers only - parts := strings.SplitN(msgStr, "\r\n\r\n", 2) - require.Len(t, parts, 2, "Email should have headers and body separated by CRLFCRLF") - headers := parts[0] - - // Count the number of header lines - there should be exactly 5: - // From, To, Subject, MIME-Version, Content-Type - headerLines := strings.Split(headers, "\r\n") - assert.Equal(t, 5, len(headerLines), - "Should have exactly 5 header lines (no injected headers)") - - // Verify no injected headers appear as separate lines - for _, line := range headerLines { - if strings.HasPrefix(line, "Bcc:") || strings.HasPrefix(line, "X-Injected:") { - t.Errorf("Injected header found: %s", line) - } - } - }) - } -} - -// TestSanitizeEmailHeader tests the sanitizeEmailHeader function directly -func TestSanitizeEmailHeader(t *testing.T) { - tests := []struct { - name string - input string - expected string - }{ - {"clean string", "Normal Subject", "Normal Subject"}, - {"CR removal", "Subject\rInjected", "SubjectInjected"}, - {"LF removal", "Subject\nInjected", "SubjectInjected"}, - {"CRLF removal", "Subject\r\nBcc: evil@hacker.com", "SubjectBcc: evil@hacker.com"}, - {"null byte removal", "Subject\x00Hidden", "SubjectHidden"}, - {"tab removal", "Subject\tTabbed", "SubjectTabbed"}, - {"multiple control chars", "A\r\n\x00\x1f\x7fB", "AB"}, - {"empty string", "", ""}, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - result := sanitizeEmailHeader(tc.input) - assert.Equal(t, tc.expected, result) - }) - } -} - -// TestValidateEmailAddress tests email address validation -func TestValidateEmailAddress(t *testing.T) { +func TestParseEmailAddressForHeader(t *testing.T) { tests := []struct { name string email string @@ -270,7 +180,7 @@ func TestValidateEmailAddress(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - err := validateEmailAddress(tc.email) + _, err := parseEmailAddressForHeader("to", tc.email) if tc.wantErr { assert.Error(t, err) } else { @@ -280,11 +190,47 @@ func TestValidateEmailAddress(t *testing.T) { } } -// TestMailService_SMTPDotStuffing tests SMTP dot-stuffing to prevent email injection (CWE-93) +func TestMailService_BuildEmail_RejectsCRLFInSubject(t *testing.T) { + db := setupMailTestDB(t) + svc := NewMailService(db) + + fromAddr, err := mail.ParseAddress("sender@example.com") + require.NoError(t, err) + toAddr, err := mail.ParseAddress("recipient@example.com") + require.NoError(t, err) + + _, err = svc.buildEmail(fromAddr, toAddr, nil, "Normal\r\nBcc: attacker@evil.com", "

Body

") + assert.Error(t, err) +} + +func TestMailService_BuildEmail_RejectsCRLFInReplyTo(t *testing.T) { + db := setupMailTestDB(t) + svc := NewMailService(db) + + fromAddr, err := mail.ParseAddress("sender@example.com") + require.NoError(t, err) + toAddr, err := mail.ParseAddress("recipient@example.com") + require.NoError(t, err) + + // Create reply-to address with CRLF injection attempt in the name field + replyToAddr := &mail.Address{ + Name: "Attacker\r\nBcc: evil@example.com", + Address: "attacker@example.com", + } + + _, err = svc.buildEmail(fromAddr, toAddr, replyToAddr, "Test Subject", "

Body

") + assert.Error(t, err, "Should reject CRLF in reply-to name") +} + func TestMailService_SMTPDotStuffing(t *testing.T) { db := setupMailTestDB(t) svc := NewMailService(db) + fromAddr, err := mail.ParseAddress("from@example.com") + require.NoError(t, err) + toAddr, err := mail.ParseAddress("to@example.com") + require.NoError(t, err) + tests := []struct { name string htmlBody string @@ -314,10 +260,10 @@ func TestMailService_SMTPDotStuffing(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - msg := svc.buildEmail("from@example.com", "to@example.com", "Test", tc.htmlBody) + msg, err := svc.buildEmail(fromAddr, toAddr, nil, "Test", tc.htmlBody) + require.NoError(t, err) msgStr := string(msg) - // Extract body (everything after \r\n\r\n) parts := strings.Split(msgStr, "\r\n\r\n") require.Len(t, parts, 2, "Email should have headers and body") body := parts[1] @@ -327,7 +273,6 @@ func TestMailService_SMTPDotStuffing(t *testing.T) { } } -// TestSanitizeEmailBody tests the sanitizeEmailBody function directly func TestSanitizeEmailBody(t *testing.T) { tests := []struct { name string @@ -368,12 +313,26 @@ func TestMailService_SendEmail_NotConfigured(t *testing.T) { assert.Contains(t, err.Error(), "not configured") } -// TestSMTPConfigSerialization ensures config fields are properly stored +func TestMailService_SendEmail_RejectsCRLFInSubject(t *testing.T) { + db := setupMailTestDB(t) + svc := NewMailService(db) + + config := &SMTPConfig{ + Host: "smtp.example.com", + Port: 587, + FromAddress: "noreply@example.com", + } + require.NoError(t, svc.SaveSMTPConfig(config)) + + err := svc.SendEmail("recipient@example.com", "Hello\r\nBcc: evil@example.com", "Body") + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid subject") +} + func TestSMTPConfigSerialization(t *testing.T) { db := setupMailTestDB(t) svc := NewMailService(db) - // Test with special characters in password config := &SMTPConfig{ Host: "smtp.example.com", Port: 587, @@ -393,19 +352,15 @@ func TestSMTPConfigSerialization(t *testing.T) { assert.Equal(t, config.FromAddress, retrieved.FromAddress) } -// TestMailService_SendInvite tests the invite email template func TestMailService_SendInvite_Template(t *testing.T) { db := setupMailTestDB(t) svc := NewMailService(db) - // We can't actually send email, but we can verify the method doesn't panic - // and returns appropriate error when SMTP is not configured err := svc.SendInvite("test@example.com", "abc123token", "TestApp", "https://example.com") assert.Error(t, err) assert.Contains(t, err.Error(), "not configured") } -// Benchmark tests func BenchmarkMailService_IsConfigured(b *testing.B) { db, _ := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{ Logger: logger.Default.LogMode(logger.Silent), @@ -432,33 +387,45 @@ func BenchmarkMailService_BuildEmail(b *testing.B) { }) svc := NewMailService(db) + fromAddr, _ := mail.ParseAddress("sender@example.com") + toAddr, _ := mail.ParseAddress("recipient@example.com") + b.ResetTimer() for i := 0; i < b.N; i++ { - svc.buildEmail( - "sender@example.com", - "recipient@example.com", - "Test Subject", - "Test Body", - ) + _, _ = svc.buildEmail(fromAddr, toAddr, nil, "Test Subject", "Test Body") } } -// Integration test placeholder - this would use a real SMTP server +func TestMailService_SendInvite_InvalidBaseURL_CRLF(t *testing.T) { + db := setupMailTestDB(t) + svc := NewMailService(db) + + err := svc.SendInvite("test@example.com", "token123", "TestApp", "https://example.com\r\nBcc: attacker@example.com") + assert.Error(t, err) + assert.Contains(t, err.Error(), "baseURL") +} + +func TestMailService_SendInvite_InvalidBaseURL_Path(t *testing.T) { + db := setupMailTestDB(t) + svc := NewMailService(db) + + err := svc.SendInvite("test@example.com", "token123", "TestApp", "https://example.com/sneaky") + assert.Error(t, err) + assert.Contains(t, err.Error(), "baseURL") +} + func TestMailService_Integration(t *testing.T) { if testing.Short() { t.Skip("Skipping integration test in short mode") } - // This test would connect to a real SMTP server (like MailHog) for integration testing t.Skip("Integration test requires SMTP server") } -// Test for expired invite token handling in SendInvite func TestMailService_SendInvite_TokenFormat(t *testing.T) { db := setupMailTestDB(t) svc := NewMailService(db) - // Save SMTP config so we can test template generation config := &SMTPConfig{ Host: "smtp.example.com", Port: 587, @@ -466,28 +433,21 @@ func TestMailService_SendInvite_TokenFormat(t *testing.T) { } svc.SaveSMTPConfig(config) - // The SendInvite will fail at SMTP connection, but we're testing that - // the function correctly constructs the invite URL err := svc.SendInvite("test@example.com", "token123", "Charon", "https://charon.local/") - assert.Error(t, err) // Will error on SMTP connection + assert.Error(t, err) - // Test with trailing slash handling err = svc.SendInvite("test@example.com", "token123", "Charon", "https://charon.local") - assert.Error(t, err) // Will error on SMTP connection + assert.Error(t, err) } -// Add timeout handling test -// Note: Skipped as in-memory SQLite doesn't support concurrent writes well func TestMailService_SaveSMTPConfig_Concurrent(t *testing.T) { t.Skip("In-memory SQLite doesn't support concurrent writes - test real DB in integration") } -// TestMailService_SendEmail_InvalidRecipient tests email sending with invalid recipient func TestMailService_SendEmail_InvalidRecipient(t *testing.T) { db := setupMailTestDB(t) svc := NewMailService(db) - // Configure SMTP config := &SMTPConfig{ Host: "smtp.example.com", Port: 587, @@ -495,18 +455,15 @@ func TestMailService_SendEmail_InvalidRecipient(t *testing.T) { } require.NoError(t, svc.SaveSMTPConfig(config)) - // Try sending with invalid recipient err := svc.SendEmail("invalid\r\nemail", "Subject", "Body") assert.Error(t, err) assert.Contains(t, err.Error(), "invalid recipient") } -// TestMailService_SendEmail_InvalidFromAddress tests email sending with invalid from address func TestMailService_SendEmail_InvalidFromAddress(t *testing.T) { db := setupMailTestDB(t) svc := NewMailService(db) - // Configure SMTP with invalid from address config := &SMTPConfig{ Host: "smtp.example.com", Port: 587, @@ -514,13 +471,11 @@ func TestMailService_SendEmail_InvalidFromAddress(t *testing.T) { } require.NoError(t, svc.SaveSMTPConfig(config)) - // Try sending email - should fail on invalid from address err := svc.SendEmail("test@example.com", "Subject", "Body") assert.Error(t, err) assert.Contains(t, err.Error(), "invalid from address") } -// TestMailService_SendEmail_EncryptionModes tests different encryption modes func TestMailService_SendEmail_EncryptionModes(t *testing.T) { db := setupMailTestDB(t) svc := NewMailService(db) @@ -547,10 +502,144 @@ func TestMailService_SendEmail_EncryptionModes(t *testing.T) { } require.NoError(t, svc.SaveSMTPConfig(config)) - // This will fail at connection/lookup time, but we're testing the path selection err := svc.SendEmail("recipient@example.com", "Test", "Body") assert.Error(t, err) - // Should fail on connection or lookup + }) + } +} + +// TestMailService_SendEmail_CRLFInjection_Comprehensive tests CRLF injection prevention +// across all email header fields (CodeQL go/email-injection remediation) +func TestMailService_SendEmail_CRLFInjection_Comprehensive(t *testing.T) { + db := setupMailTestDB(t) + svc := NewMailService(db) + + config := &SMTPConfig{ + Host: "smtp.example.com", + Port: 587, + FromAddress: "noreply@example.com", + Encryption: "starttls", + } + require.NoError(t, svc.SaveSMTPConfig(config)) + + tests := []struct { + name string + to string + subject string + fromAddress string + description string + }{ + { + name: "CRLF in recipient address", + to: "victim@example.com\r\nBcc: attacker@evil.com", + subject: "Normal Subject", + fromAddress: "noreply@example.com", + description: "Reject CRLF in To header", + }, + { + name: "CRLF in subject line", + to: "victim@example.com", + subject: "Test\r\nBcc: attacker@evil.com", + fromAddress: "noreply@example.com", + description: "Reject CRLF in Subject header", + }, + { + name: "LF only in recipient", + to: "victim@example.com\nBcc: attacker@evil.com", + subject: "Normal Subject", + fromAddress: "noreply@example.com", + description: "Reject LF in To header", + }, + { + name: "LF only in subject", + to: "victim@example.com", + subject: "Test\nBcc: attacker@evil.com", + fromAddress: "noreply@example.com", + description: "Reject LF in Subject header", + }, + { + name: "CR only in recipient", + to: "victim@example.com\rBcc: attacker@evil.com", + subject: "Normal Subject", + fromAddress: "noreply@example.com", + description: "Reject CR in To header", + }, + { + name: "multiple CRLF sequences", + to: "victim@example.com", + subject: "Test\r\nBcc: evil1@attacker.com\r\nCc: evil2@attacker.com", + fromAddress: "noreply@example.com", + description: "Reject multiple CRLF attempts", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Update config with potentially malicious from address if specified + if tc.fromAddress != config.FromAddress { + testConfig := *config + testConfig.FromAddress = tc.fromAddress + require.NoError(t, svc.SaveSMTPConfig(&testConfig)) + } + + err := svc.SendEmail(tc.to, tc.subject, "

Normal body

") + assert.Error(t, err, tc.description) + assert.Contains(t, err.Error(), "invalid", "Error should indicate invalid input") + }) + } +} + +// TestMailService_SendInvite_CRLFInjection tests CRLF injection prevention in invite emails +func TestMailService_SendInvite_CRLFInjection(t *testing.T) { + db := setupMailTestDB(t) + svc := NewMailService(db) + + config := &SMTPConfig{ + Host: "smtp.example.com", + Port: 587, + FromAddress: "noreply@example.com", + Encryption: "starttls", + } + require.NoError(t, svc.SaveSMTPConfig(config)) + + tests := []struct { + name string + email string + token string + appName string + baseURL string + description string + }{ + { + name: "CRLF in email address", + email: "victim@example.com\r\nBcc: attacker@evil.com", + token: "token123", + appName: "TestApp", + baseURL: "https://example.com", + description: "Reject CRLF in invite email address", + }, + { + name: "CRLF in baseURL", + email: "user@example.com", + token: "token123", + appName: "TestApp", + baseURL: "https://example.com\r\nBcc: attacker@evil.com", + description: "Reject CRLF in invite baseURL", + }, + { + name: "CRLF in app name (subject)", + email: "user@example.com", + token: "token123", + appName: "TestApp\r\nBcc: attacker@evil.com", + baseURL: "https://example.com", + description: "Reject CRLF in app name used in subject", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := svc.SendInvite(tc.email, tc.token, tc.appName, tc.baseURL) + assert.Error(t, err, tc.description) }) } } diff --git a/docs/implementation/WORKSTREAM_C_CROWDSEC_GO_VERSION_FIX.md b/docs/implementation/WORKSTREAM_C_CROWDSEC_GO_VERSION_FIX.md new file mode 100644 index 00000000..e6f3230b --- /dev/null +++ b/docs/implementation/WORKSTREAM_C_CROWDSEC_GO_VERSION_FIX.md @@ -0,0 +1,76 @@ +# Workstream C: CrowdSec Go Version Fix + +**Date:** 2026-01-10 +**Issue:** CrowdSec binaries built with Go 1.25.1 containing 4 HIGH CVEs +**Solution:** Pin CrowdSec builder to Go 1.25.5+ + +## Problem + +Trivy scan identified that the CrowdSec binaries (`crowdsec` and `cscli`) embedded in the container image were built with Go 1.25.1, which has 4 HIGH severity CVEs: +- CVE-2025-58183 +- CVE-2025-58186 +- CVE-2025-58187 +- CVE-2025-61729 + +The CrowdSec builder stage in the Dockerfile was using `golang:1.25-alpine`, which resolved to the vulnerable Go 1.25.1 version. + +## Solution + +Updated the `CrowdSec Builder` stage in the Dockerfile to explicitly pin to Go 1.25.5: + +```dockerfile +# Before: +FROM --platform=$BUILDPLATFORM golang:1.25-alpine AS crowdsec-builder + +# After: +# renovate: datasource=docker depName=golang versioning=docker +FROM --platform=$BUILDPLATFORM golang:1.25.5-alpine AS crowdsec-builder +``` + +## Changes Made + +### File: `Dockerfile` + +**Line ~275-279:** Updated the CrowdSec builder stage base image +- Changed from: `golang:1.25-alpine` (resolves to 1.25.1) +- Changed to: `golang:1.25.5-alpine` (fixed version) +- Added Renovate annotation to track future Go version updates + +## Impact + +- **Security:** Eliminates 4 HIGH CVEs in the CrowdSec binaries +- **Build Process:** No changes to build logic, only base image version +- **CrowdSec Version:** Remains at v1.7.4 (no version change needed) +- **Compatibility:** No breaking changes; CrowdSec functionality unchanged + +## Verification + +After this change, the following validations should be performed: + +1. **Rebuild the image** (no-cache recommended): + ```bash + # Use task: Build & Run: Local Docker Image No-Cache + ``` + +2. **Run Trivy scan** on the rebuilt image: + ```bash + # Use task: Security: Trivy Scan + ``` + +3. **Expected outcome:** + - Trivy image scan should report **0 HIGH/CRITICAL** vulnerabilities + - CrowdSec binaries should be built with Go 1.25.5+ + - All CrowdSec functionality should remain operational + +## Related + +- **Plan:** [docs/plans/current_spec.md](../plans/current_spec.md) - Workstream C +- **CVE List:** Go 1.25.1 stdlib vulnerabilities (CVE-2025-58183, CVE-2025-58186, CVE-2025-58187, CVE-2025-61729) +- **Dependencies:** CrowdSec v1.7.4 (no change) +- **Next Step:** QA validation after image rebuild + +## Notes + +- The Backend Builder stage already uses `golang:1.25-alpine` but may resolve to a patched minor version. If needed, it can be pinned similarly. +- Renovate will track the pinned `golang:1.25.5-alpine` image and suggest updates when newer patch versions are available. +- The explicit version pin ensures reproducible builds and prevents accidental rollback to vulnerable versions. diff --git a/docs/reports/qa_report.md b/docs/reports/qa_report.md index 8fc49447..30b0cbb2 100644 --- a/docs/reports/qa_report.md +++ b/docs/reports/qa_report.md @@ -113,3 +113,29 @@ Per instruction: **no fixes were made**. Suggested remediation steps: - Backend coverage profile: `backend/coverage.txt` - CodeQL results: `codeql-results-go.sarif`, `codeql-results-js.sarif`, `codeql-results-javascript.sarif` - Trivy results: `trivy-scan-output.txt`, `trivy-image-scan.txt` + +## Trivy triage (2026-01-10) + +**Task rerun**: VS Code task “Security: Trivy Scan” + +**Primary artifact (current task output)**: +- `.trivy_logs/trivy-report.txt` + +**What the task is actually scanning**: +- Image scan only (`trivy image --severity CRITICAL,HIGH charon:local`), not a filesystem/repo scan. + +**Current HIGH/CRITICAL summary (from `.trivy_logs/trivy-report.txt`)**: +- **CRITICAL=0, HIGH=8** +- All HIGH findings are in **built image contents**, specifically: + - `usr/local/bin/crowdsec` (**HIGH=4**) and `usr/local/bin/cscli` (**HIGH=4**) + - Vulnerabilities are attributed to **Go stdlib** in those binaries (built with Go `v1.25.1`): + - `CVE-2025-58183`, `CVE-2025-58186`, `CVE-2025-58187`, `CVE-2025-61729` + +**Attribution**: +- Repo-tracked source paths: **none** (this task does not scan the repo filesystem) +- Generated artifacts/caches: **none** (this task does not scan the repo filesystem) +- Built image contents: **YES** (CrowdSec binaries embed vulnerable Go stdlib) + +**What must be fixed next (no fixes applied here)**: +- **Dockerfile/CrowdSec bump**: update the CrowdSec build stage/version/toolchain so `crowdsec` and `cscli` are built with a Go version that includes the fixes (per Trivy, fixed in Go `1.25.2+`, `1.25.3+`, and `1.25.5+` depending on CVE), then rebuild `charon:local` and rerun Trivy. +- If DoD is intended to gate repo dependencies too, consider **scan-scope alignment** (add a separate Trivy filesystem scan of repo-tracked paths with excludes for workspace caches like `.cache/`, `codeql-db*/`, and scan outputs). diff --git a/frontend/src/pages/__tests__/ProxyHosts-extra.test.tsx b/frontend/src/pages/__tests__/ProxyHosts-extra.test.tsx index ec830b71..4fb560a3 100644 --- a/frontend/src/pages/__tests__/ProxyHosts-extra.test.tsx +++ b/frontend/src/pages/__tests__/ProxyHosts-extra.test.tsx @@ -249,7 +249,8 @@ describe('ProxyHosts page extra tests', () => { renderWithProviders() await waitFor(() => expect(screen.getByText('link.example.com')).toBeInTheDocument()) - const link = screen.getByRole('link', { name: /link.example.com/ }) + // Use exact string match to avoid incomplete hostname regex (CodeQL js/incomplete-hostname-regexp) + const link = screen.getByRole('link', { name: 'link.example.com' }) await userEvent.click(link) expect(openSpy).toHaveBeenCalled() openSpy.mockRestore()