diff --git a/internal/server/handlers_admin.go b/internal/server/handlers_admin.go index bd364fb..a151b76 100644 --- a/internal/server/handlers_admin.go +++ b/internal/server/handlers_admin.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "encoding/json" "fmt" + "html" "net/http" "strings" "time" @@ -113,8 +114,8 @@ td{padding:0.5rem;border-bottom:1px solid #0f3460}a{color:#4ecca3}` + u["username"].(string) + `` + u["email"].(string) + - `` + confirmed + `` + blocked + `` + u["created_at"].(string) + ``)) + w.Write([]byte(`` + html.EscapeString(u["username"].(string)) + `` + html.EscapeString(u["email"].(string)) + + `` + confirmed + `` + blocked + `` + html.EscapeString(u["created_at"].(string)) + ``)) } w.Write([]byte(``)) } @@ -148,8 +149,8 @@ td{padding:0.5rem;border-bottom:1px solid #0f3460}a{color:#4ecca3}` + name + `` + id + - `` + clientVer + `` + lastSeen + `` + revokedAt + `` + createdAt + ``)) + w.Write([]byte(`` + html.EscapeString(name) + `` + html.EscapeString(id) + + `` + html.EscapeString(clientVer) + `` + html.EscapeString(lastSeen) + `` + html.EscapeString(revokedAt) + `` + html.EscapeString(createdAt) + ``)) } w.Write([]byte(``)) } diff --git a/internal/server/handlers_web_user.go b/internal/server/handlers_web_user.go index e94d22f..8d6c591 100644 --- a/internal/server/handlers_web_user.go +++ b/internal/server/handlers_web_user.go @@ -4,8 +4,11 @@ import ( "crypto/rand" "encoding/hex" "fmt" + "html" "log" "net/http" + "net/url" + "strconv" "strings" "time" @@ -187,8 +190,8 @@ func (s *Server) handleUserWebReset(w http.ResponseWriter, r *http.Request) { return } w.Header().Set("Content-Type", "text/html; charset=utf-8") - html := strings.ReplaceAll(resetPasswordHTML(locale), "{TOKEN}", token) - w.Write([]byte(html)) + page := strings.ReplaceAll(resetPasswordHTML(locale), "{TOKEN}", html.EscapeString(token)) + w.Write([]byte(page)) case "POST": if err := r.ParseForm(); err != nil { jsonErr(w, 400, "bad form") @@ -204,12 +207,12 @@ func (s *Server) handleUserWebReset(w http.ResponseWriter, r *http.Request) { } if err := validatePassword(newPass); err != "" { w.Header().Set("Content-Type", "text/html; charset=utf-8") - w.Write([]byte(errorPageHTML(locale, t(locale, "common.error"), string(err), "/reset?token="+token))) + w.Write([]byte(errorPageHTML(locale, t(locale, "common.error"), string(err), "/reset?token="+url.QueryEscape(token)))) return } if newPass != confirm { w.Header().Set("Content-Type", "text/html; charset=utf-8") - w.Write([]byte(errorPageHTML(locale, t(locale, "common.error"), t(locale, "server.passwordsDoNotMatch"), "/reset?token="+token))) + w.Write([]byte(errorPageHTML(locale, t(locale, "common.error"), t(locale, "server.passwordsDoNotMatch"), "/reset?token="+url.QueryEscape(token)))) return } userID, err := s.resetPasswordWithToken(token, newPass) @@ -310,7 +313,7 @@ func (s *Server) handleUserDashboard(w http.ResponseWriter, r *http.Request) { created = created[:10] } status := "" + t(locale, "userDashboard.active") + "" - revokeBtn := fmt.Sprintf(``, d.ID, t(locale, "userDashboard.revoke")) + revokeBtn := fmt.Sprintf(``, html.EscapeString(strconv.Quote(d.ID)), t(locale, "userDashboard.revoke")) if d.RevokedAt != "" { status = "" + t(locale, "userDashboard.revoked") + "" revokeBtn = "" @@ -321,11 +324,11 @@ func (s *Server) handleUserDashboard(w http.ResponseWriter, r *http.Request) { %s %s %s %s - `, d.Name, status, created, ls, d.ClientVer, revokeBtn) + `, html.EscapeString(d.Name), status, html.EscapeString(created), html.EscapeString(ls), html.EscapeString(d.ClientVer), revokeBtn) } } - w.Write([]byte(userDashboardHTML(locale, username, deviceRows))) + w.Write([]byte(userDashboardHTML(locale, html.EscapeString(username), deviceRows))) } func (s *Server) handleUserWebLogout(w http.ResponseWriter, r *http.Request) { diff --git a/internal/server/server_test.go b/internal/server/server_test.go index d2c7c6c..0bde9d0 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -4,6 +4,7 @@ import ( "bytes" "database/sql" "encoding/json" + "html" "net/http" "net/http/httptest" "net/url" @@ -372,6 +373,74 @@ func TestWebResetRejectsExpiredToken(t *testing.T) { } } +func TestServerRenderedPagesEscapeStoredValues(t *testing.T) { + s, _ := newSyncHTTPServer(t) + defer s.Close() + + const maliciousText = `` + const maliciousDeviceID = `device');alert(1);//` + now := time.Now().UTC().Format(time.RFC3339) + if _, err := s.db.Exec(`INSERT INTO server_users + (id, username, email, password_hash, confirmed, created_at) + VALUES (?, ?, ?, ?, 1, ?)`, "user-a", maliciousText, maliciousText+"@example.test", "unused", now); err != nil { + t.Fatalf("insert user: %v", err) + } + if _, err := s.db.Exec(`INSERT INTO server_devices + (id, name, api_key, user_id, vault_id, client_version, last_seen, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, maliciousDeviceID, maliciousText, "device-key", "user-a", "vault-a", maliciousText, now, now); err != nil { + t.Fatalf("insert device: %v", err) + } + if _, err := s.db.Exec("INSERT INTO server_user_devices (user_id, device_id) VALUES (?, ?)", "user-a", maliciousDeviceID); err != nil { + t.Fatalf("link user device: %v", err) + } + + tests := []struct { + name string + path string + cookie *http.Cookie + containsDevID bool + }{ + { + name: "user dashboard", + path: "/dashboard", + cookie: &http.Cookie{Name: "user_session", Value: s.userTokens.Create("user-a")}, + containsDevID: true, + }, + { + name: "admin users", + path: "/admin/users", + cookie: &http.Cookie{Name: "admin_session", Value: s.tokens.Create()}, + }, + { + name: "admin devices", + path: "/admin/devices", + cookie: &http.Cookie{Name: "admin_session", Value: s.tokens.Create()}, + containsDevID: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, tt.path, nil) + req.AddCookie(tt.cookie) + res := httptest.NewRecorder() + s.mux.ServeHTTP(res, req) + if res.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", res.Code) + } + body := res.Body.String() + if strings.Contains(body, maliciousText) { + t.Fatalf("page contains unescaped stored text: %s", body) + } + if !strings.Contains(body, html.EscapeString(maliciousText)) { + t.Fatalf("page does not contain escaped stored text: %s", body) + } + if tt.containsDevID && strings.Contains(body, maliciousDeviceID) { + t.Fatalf("page contains unescaped device ID: %s", body) + } + }) + } +} + func TestNewServerMigratesLegacyOperationScope(t *testing.T) { dir := t.TempDir() dbPath := filepath.Join(dir, "legacy.db") diff --git a/internal/server/templates.go b/internal/server/templates.go index 47c52f5..14474e6 100644 --- a/internal/server/templates.go +++ b/internal/server/templates.go @@ -2,6 +2,7 @@ package server import ( "fmt" + "html" "strings" ) @@ -552,6 +553,9 @@ button:hover{background:#4f46e5} } func errorPageHTML(locale, title, msg, backURL string) string { + title = html.EscapeString(title) + msg = html.EscapeString(msg) + backURL = html.EscapeString(backURL) return fmt.Sprintf(`