diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 2c991fb..45990ee 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -13,3 +13,11 @@ **Learning:** `ip.IsPrivate()` and `ip.IsLoopback()` are not sufficient to block all local traffic. The concept of "Unspecified" addresses (all zeros) must also be explicitly handled when validating IPs for SSRF protection in Go. **Prevention:** When implementing a safe dialer to prevent SSRF, always include `ip.IsUnspecified()` in the list of blocked IP characteristics, in addition to private, loopback, and link-local addresses. + +## 2026-01-27 - Fix Stored XSS in HTML Output + +**Vulnerability:** The application was vulnerable to Stored Cross-Site Scripting (XSS) because it rendered HTML content from the `go-readability` library without adequate sanitization. Although `go-readability` performs some cleaning, it does not guarantee safety against all XSS vectors (e.g., `onerror` handlers). + +**Learning:** Relying solely on a parsing library for security is insufficient. Libraries designed for content extraction (like `readability`) often prioritize preserving content structure over strict security sanitization. A dedicated sanitizer is required before rendering user-controlled HTML. + +**Prevention:** We integrated `bluemonday` with `UGCPolicy` to explicitly sanitize the HTML content before it is injected into the HTML template or JSON response. This ensures that any malicious tags or attributes missed by the parser are stripped. diff --git a/api/index.go b/api/index.go index b64870e..764ab3e 100644 --- a/api/index.go +++ b/api/index.go @@ -26,6 +26,7 @@ import ( "codeberg.org/readeck/go-readability/v2" "github.com/mattn/godown" + "github.com/microcosm-cc/bluemonday" "golang.org/x/net/html" ) @@ -57,6 +58,8 @@ const Template = ` var ( DefaultTemplate = template.Must(template.New("article").Parse(Template)) ReadabilityParser = readability.NewParser() + // sanitizer is used to clean up HTML content to prevent XSS + sanitizer = bluemonday.UGCPolicy() // httpClient used for fetching remote articles with timeouts and redirect policy httpClient = &http.Client{ Transport: &http.Transport{ @@ -270,7 +273,7 @@ func formatHTML(w http.ResponseWriter, article readability.Article, contentBuf * Content template.HTML }{ Title: article.Title(), - Content: template.HTML(contentBuf.String()), + Content: template.HTML(sanitizer.Sanitize(contentBuf.String())), } if err := DefaultTemplate.Execute(w, data); err != nil { // at this point, we can't write a JSON error, so we log it @@ -297,7 +300,7 @@ func formatJSON(w http.ResponseWriter, article readability.Article, buf *bytes.B w.Header().Set("Content-Type", "application/json") if err := json.NewEncoder(w).Encode(map[string]string{ "title": article.Title(), - "content": buf.String(), + "content": sanitizer.Sanitize(buf.String()), }); err != nil { log.Printf("error encoding json: %v", err) } diff --git a/api/reconstruct_test.go b/api/reconstruct_test.go index 369de89..22d44ff 100644 --- a/api/reconstruct_test.go +++ b/api/reconstruct_test.go @@ -63,12 +63,12 @@ func TestReconstructTargetURL(t *testing.T) { gotU, _ := url.Parse(got) expU, _ := url.Parse(tt.expected) - if gotU == nil || expU == nil { - if got != tt.expected { - t.Errorf("reconstructTargetURL() = %v, want %v", got, tt.expected) - } - return - } + if gotU == nil || expU == nil { + if got != tt.expected { + t.Errorf("reconstructTargetURL() = %v, want %v", got, tt.expected) + } + return + } if gotU.Scheme != expU.Scheme || gotU.Host != expU.Host || gotU.Path != expU.Path { t.Errorf("reconstructTargetURL() base mismatch = %v, want %v", got, tt.expected) diff --git a/api/xss_test.go b/api/xss_test.go new file mode 100644 index 0000000..a064a03 --- /dev/null +++ b/api/xss_test.go @@ -0,0 +1,80 @@ +package handler + +import ( + "bytes" + "context" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" +) + +func TestXSSVulnerability(t *testing.T) { + // Malicious HTML payload + htmlBody := ` + + +
Safe content
+ +