Răsfoiți Sursa

feat(response): change error response content type to plain text and escape HTML

Adding another layer of security in addition to the existing CSP cannot
hurt.
Frédéric Guillot 11 luni în urmă
părinte
comite
036704b3e4

+ 5 - 4
internal/http/response/html/html.go

@@ -4,6 +4,7 @@
 package html // import "miniflux.app/v2/internal/http/response/html"
 
 import (
+	"html"
 	"log/slog"
 	"net/http"
 
@@ -38,9 +39,9 @@ func ServerError(w http.ResponseWriter, r *http.Request, err error) {
 	builder := response.New(w, r)
 	builder.WithStatus(http.StatusInternalServerError)
 	builder.WithHeader("Content-Security-Policy", response.ContentSecurityPolicyForUntrustedContent)
-	builder.WithHeader("Content-Type", "text/html; charset=utf-8")
+	builder.WithHeader("Content-Type", "text/plain; charset=utf-8")
 	builder.WithHeader("Cache-Control", "no-cache, max-age=0, must-revalidate, no-store")
-	builder.WithBody(err)
+	builder.WithBody(html.EscapeString(err.Error()))
 	builder.Write()
 }
 
@@ -62,9 +63,9 @@ func BadRequest(w http.ResponseWriter, r *http.Request, err error) {
 	builder := response.New(w, r)
 	builder.WithStatus(http.StatusBadRequest)
 	builder.WithHeader("Content-Security-Policy", response.ContentSecurityPolicyForUntrustedContent)
-	builder.WithHeader("Content-Type", "text/html; charset=utf-8")
+	builder.WithHeader("Content-Type", "text/plain; charset=utf-8")
 	builder.WithHeader("Cache-Control", "no-cache, max-age=0, must-revalidate, no-store")
-	builder.WithBody(err)
+	builder.WithBody(html.EscapeString(err.Error()))
 	builder.Write()
 }
 

+ 6 - 6
internal/http/response/html/html_test.go

@@ -58,7 +58,7 @@ func TestServerErrorResponse(t *testing.T) {
 	w := httptest.NewRecorder()
 
 	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		ServerError(w, r, errors.New("Some error"))
+		ServerError(w, r, errors.New("Some error with injected HTML <script>alert('XSS')</script>"))
 	})
 
 	handler.ServeHTTP(w, r)
@@ -69,13 +69,13 @@ func TestServerErrorResponse(t *testing.T) {
 		t.Fatalf(`Unexpected status code, got %d instead of %d`, resp.StatusCode, expectedStatusCode)
 	}
 
-	expectedBody := `Some error`
+	expectedBody := `Some error with injected HTML &lt;script&gt;alert(&#39;XSS&#39;)&lt;/script&gt;`
 	actualBody := w.Body.String()
 	if actualBody != expectedBody {
 		t.Fatalf(`Unexpected body, got %s instead of %s`, actualBody, expectedBody)
 	}
 
-	expectedContentType := "text/html; charset=utf-8"
+	expectedContentType := "text/plain; charset=utf-8"
 	actualContentType := resp.Header.Get("Content-Type")
 	if actualContentType != expectedContentType {
 		t.Fatalf(`Unexpected content type, got %q instead of %q`, actualContentType, expectedContentType)
@@ -91,7 +91,7 @@ func TestBadRequestResponse(t *testing.T) {
 	w := httptest.NewRecorder()
 
 	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		BadRequest(w, r, errors.New("Some error"))
+		BadRequest(w, r, errors.New("Some error with injected HTML <script>alert('XSS')</script>"))
 	})
 
 	handler.ServeHTTP(w, r)
@@ -102,13 +102,13 @@ func TestBadRequestResponse(t *testing.T) {
 		t.Fatalf(`Unexpected status code, got %d instead of %d`, resp.StatusCode, expectedStatusCode)
 	}
 
-	expectedBody := `Some error`
+	expectedBody := `Some error with injected HTML &lt;script&gt;alert(&#39;XSS&#39;)&lt;/script&gt;`
 	actualBody := w.Body.String()
 	if actualBody != expectedBody {
 		t.Fatalf(`Unexpected body, got %s instead of %s`, actualBody, expectedBody)
 	}
 
-	expectedContentType := "text/html; charset=utf-8"
+	expectedContentType := "text/plain; charset=utf-8"
 	actualContentType := resp.Header.Get("Content-Type")
 	if actualContentType != expectedContentType {
 		t.Fatalf(`Unexpected content type, got %q instead of %q`, actualContentType, expectedContentType)