Skip to content

🛡️ Sentinel: fix XSS vulnerability in HTTP server dashboard #90

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
filmil wants to merge 1 commit into from
Closed

🛡️ Sentinel: fix XSS vulnerability in HTTP server dashboard #90

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions internal/server/grpc_status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ func (s *HTTPServer) handleGRPC(w http.ResponseWriter, r *http.Request) {

members, err := s.store.GetMembers()
if err != nil {
data.ErrorMsg = template.HTML("<div class='alert alert-danger'>Failed to retrieve members</div>")
data.ErrorMsg = "Failed to retrieve members"
s.renderGRPCStatus(w, data)
return
}

selfInfo, ok := members[agentID]
if !ok || selfInfo.GRPCAddr == "" {
data.ErrorMsg = template.HTML("<div class='alert alert-danger'>gRPC address not found for self</div>")
data.ErrorMsg = "gRPC address not found for self"
s.renderGRPCStatus(w, data)
return
}
Expand All @@ -51,7 +51,7 @@ func (s *HTTPServer) handleGRPC(w http.ResponseWriter, r *http.Request) {

conn, err := grpc.NewClient(selfInfo.GRPCAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
data.ErrorMsg = template.HTML(fmt.Sprintf("<div class='alert alert-danger'>Failed to connect to local gRPC channelz: %v</div>", html.EscapeString(err.Error())))
data.ErrorMsg = fmt.Sprintf("Failed to connect to local gRPC channelz: %v", err)
s.renderGRPCStatus(w, data)
return
}
Expand All @@ -62,18 +62,18 @@ func (s *HTTPServer) handleGRPC(w http.ResponseWriter, r *http.Request) {
// Get Servers
servers, err := s.fetchGRPCServers(ctx, client)
if err != nil {
data.ErrorMsg = template.HTML(fmt.Sprintf("<div class='alert alert-warning'>Failed to get servers: %v</div>", html.EscapeString(err.Error())))
data.ErrorMsg = fmt.Sprintf("Failed to get servers: %v", err)
}
data.Servers = servers

// Get Top Channels
channels, err := s.fetchGRPCChannels(ctx, client)
if err != nil {
errMsg := fmt.Sprintf("<div class='alert alert-warning'>Failed to get top channels: %v</div>", html.EscapeString(err.Error()))
errMsg := fmt.Sprintf("Failed to get top channels: %v", err)
if data.ErrorMsg != "" {
data.ErrorMsg = template.HTML(string(data.ErrorMsg) + errMsg)
data.ErrorMsg = data.ErrorMsg + "; " + errMsg
} else {
data.ErrorMsg = template.HTML(errMsg)
data.ErrorMsg = errMsg
}
}
data.Channels = channels
Expand Down
129 changes: 57 additions & 72 deletions internal/server/http_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@ type OngoingRequest struct {
//go:embed templates/*.html
var templateFS embed.FS

var templates = template.Must(template.ParseFS(templateFS, "templates/*.html"))
var templates = template.Must(template.New("").Funcs(template.FuncMap{
"sanitizeURL": sanitizeURL,
"wrapString": wrapString,
}).ParseFS(templateFS, "templates/*.html"))

// HTTPServer provides a web dashboard for inspecting agent state and issuing commands.
type BaseData struct {
Expand All @@ -61,24 +64,27 @@ type BaseData struct {
}

type Participant struct {
IDLabel template.HTML
ID string
IsSelf bool
ShortName string
GRPCAddr string
HTTPLink template.HTML
HTTPURL string
}

type IndexData struct {
BaseData
AgentID template.HTML
AgentID string
AgentCert string
KeysCount int
Participants []Participant
}

type Peer struct {
Label template.HTML
Name string
IsSelf bool
AgentID string
Endpoints template.HTML
GRPCAddr string
HTTPURL string
}

type Message struct {
Expand All @@ -98,12 +104,12 @@ type MessagesData struct {
}

type KnownPeer struct {
ID string
Name string
GRPCURL string
HTTPLink template.HTML
HasCert bool
CertPEM string
ID string
Name string
GRPCURL string
HTTPURL string
HasCert bool
CertPEM string
}

type PeersData struct {
Expand All @@ -112,7 +118,7 @@ type PeersData struct {
}

type KV struct {
Key template.HTML
Key string
Value template.HTML
Type string
Version uint64
Expand Down Expand Up @@ -169,7 +175,7 @@ type GRPCChannelData struct {

type GRPCStatusData struct {
BaseData
ErrorMsg template.HTML
ErrorMsg string
Servers []GRPCServerData
Channels []GRPCChannelData
}
Expand Down Expand Up @@ -230,7 +236,15 @@ const (
<td>{{.Time}}</td>
<td>{{.Type}}</td>
<td><code>{{.Key}}</code></td>
<td>{{.StatusHTML}}</td>
<td>
{{if not .Finished}}
<span class="badge bg-secondary">Ongoing...</span>
{{else if .Success}}
<span class="badge bg-success">Success</span>
{{else}}
<span class="badge bg-danger">Failed</span>
{{end}}
</td>
<td><small>{{.Result}}</small></td>
</tr>
{{end}}
Expand Down Expand Up @@ -395,28 +409,18 @@ func (s *HTTPServer) prepareParticipants(members map[string]state.PeerInfo, self

var participants []Participant
for _, id := range ids {
idWrapped := fmt.Sprintf(`<div style="white-space: pre-wrap; font-family: monospace; word-break: break-all;">%s</div>`, html.EscapeString(wrapString(id, 40)))
label := idWrapped
if id == selfID {
label = fmt.Sprintf("%s <span class=\"badge bg-secondary\">self</span>", idWrapped)
}
info := members[id]

httpLink := ""
if info.HTTPURL != "" {
httpLink = fmt.Sprintf("<a href=\"%s\" class=\"text-decoration-none\" target=\"_blank\">%s</a>", html.EscapeString(sanitizeURL(info.HTTPURL)), html.EscapeString(info.HTTPURL))
} else {
httpLink = "<span class=\"text-muted\">N/A</span>"
}
grpcAddr := info.GRPCAddr
if grpcAddr == "" {
grpcAddr = "unknown"
}
participants = append(participants, Participant{
IDLabel: template.HTML(label),
ID: id,
IsSelf: id == selfID,
ShortName: info.ShortName,
GRPCAddr: grpcAddr,
HTTPLink: template.HTML(httpLink),
HTTPURL: info.HTTPURL,
})
}
return participants
Expand Down Expand Up @@ -449,7 +453,7 @@ func (s *HTTPServer) handleIndex(w http.ResponseWriter, r *http.Request) {

data := IndexData{
BaseData: base,
AgentID: template.HTML(fmt.Sprintf(`<div style="white-space: pre-wrap; font-family: monospace; word-break: break-all;">%s</div>`, html.EscapeString(wrapString(agentID, 40)))),
AgentID: agentID,
KeysCount: len(kvs),
Participants: s.prepareParticipants(members, agentID),
}
Expand Down Expand Up @@ -502,24 +506,17 @@ func (s *HTTPServer) handleMessages(w http.ResponseWriter, r *http.Request) {

for _, id := range ids {
info := members[id]
label := html.EscapeString(info.ShortName)
if id == agentID {
label = fmt.Sprintf("%s <span class=\"badge bg-secondary\">self</span>", label)
}

grpcAddr := info.GRPCAddr
if grpcAddr == "" {
grpcAddr = "unknown"
}
endpoints := fmt.Sprintf("<code>%s</code>", html.EscapeString(grpcAddr))
if info.HTTPURL != "" {
endpoints += fmt.Sprintf(" | <a href=\"%s\" target=\"_blank\">%s</a>", html.EscapeString(sanitizeURL(info.HTTPURL)), html.EscapeString(info.HTTPURL))
}

data.Peers = append(data.Peers, Peer{
Label: template.HTML(label),
AgentID: id,
Endpoints: template.HTML(endpoints),
Name: info.ShortName,
IsSelf: id == agentID,
AgentID: id,
GRPCAddr: grpcAddr,
HTTPURL: info.HTTPURL,
})
}

Expand Down Expand Up @@ -591,12 +588,6 @@ func (s *HTTPServer) handlePeers(w http.ResponseWriter, r *http.Request) {
}
grpcURL := fmt.Sprintf("grpc://%s", grpcAddr)

httpLink := ""
if info.HTTPURL != "" {
httpLink = fmt.Sprintf("<a href=\"%s\" class=\"text-decoration-none\" target=\"_blank\">%s</a>", html.EscapeString(sanitizeURL(info.HTTPURL)), html.EscapeString(info.HTTPURL))
} else {
httpLink = "<span class=\"text-muted\">N/A</span>"
}
certPEM := ""
if len(info.Certificate) > 0 {
if cert, err := identity.UnmarshalCertificate(info.Certificate); err == nil {
Expand All @@ -605,12 +596,12 @@ func (s *HTTPServer) handlePeers(w http.ResponseWriter, r *http.Request) {
}

data.Peers = append(data.Peers, KnownPeer{
ID: id,
Name: info.ShortName,
GRPCURL: grpcURL,
HTTPLink: template.HTML(httpLink),
HasCert: len(info.Certificate) > 0,
CertPEM: certPEM,
ID: id,
Name: info.ShortName,
GRPCURL: grpcURL,
HTTPURL: info.HTTPURL,
HasCert: len(info.Certificate) > 0,
CertPEM: certPEM,
})
}

Expand Down Expand Up @@ -640,7 +631,7 @@ func (s *HTTPServer) handleStore(w http.ResponseWriter, r *http.Request) {

for _, kv := range kvs {
data.KVs = append(data.KVs, KV{
Key: template.HTML(fmt.Sprintf(`<div style="white-space: pre-wrap; font-family: monospace; word-break: break-all;">%s</div>`, html.EscapeString(wrapString(kv.Key, 40)))),
Key: kv.Key,
Value: template.HTML(maybeJSONToTable(kv.Value)),
Type: kv.Type,
Version: kv.Version,
Expand Down Expand Up @@ -722,32 +713,26 @@ func (s *HTTPServer) handleUserAPI(w http.ResponseWriter, r *http.Request) {
ongoingHTML := ""
if len(ongoing) > 0 {
type renderEntry struct {
Time string
Type string
Key string
StatusHTML template.HTML
Result string
Time string
Type string
Key string
Finished bool
Success bool
Result string
}
var entries []renderEntry
for _, r := range ongoing {
status := "<span class=\"badge bg-secondary\">Ongoing...</span>"
if r.Finished {
if r.Success {
status = "<span class=\"badge bg-success\">Success</span>"
} else {
status = "<span class=\"badge bg-danger\">Failed</span>"
}
}
resultText := r.Result
if len(resultText) > 100 {
resultText = resultText[:97] + "..."
}
entries = append(entries, renderEntry{
Time: r.StartTime.Format("15:04:05"),
Type: r.Type,
Key: r.Key,
StatusHTML: template.HTML(status),
Result: resultText,
Time: r.StartTime.Format("15:04:05"),
Type: r.Type,
Key: r.Key,
Finished: r.Finished,
Success: r.Success,
Result: resultText,
})
}
var sb strings.Builder
Expand Down
58 changes: 58 additions & 0 deletions internal/server/http_server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,61 @@ func TestHTTPServer_handleSystem(t *testing.T) {
t.Errorf("expected 'Go Runtime Statistics' in body")
}
}

func TestHTTPServer_XSSProtection(t *testing.T) {
server, cleanup := setupTestServer(t)
defer cleanup()

// Inject malicious peer info
maliciousID := "attacker-id<script>alert('id')</script>"
maliciousName := "Attacker<script>alert('name')</script>"
maliciousURL := "javascript:alert('xss')"

server.store.UpdatePeerInfo(maliciousID, state.PeerInfo{
ShortName: maliciousName,
HTTPURL: maliciousURL,
GRPCAddr: "127.0.0.1:50051",
})

t.Run("IndexPage", func(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, "/", nil)
w := httptest.NewRecorder()
server.handleIndex(w, req)
body := w.Body.String()

if strings.Contains(body, maliciousID) {
t.Errorf("Body contains raw malicious ID, should be escaped")
}
if strings.Contains(body, maliciousName) {
t.Errorf("Body contains raw malicious Name, should be escaped")
}
if strings.Contains(body, maliciousURL) && !strings.Contains(body, "#") {
t.Errorf("Body contains raw malicious URL, should be sanitized to #")
}

escapedID := html.EscapeString(maliciousID)
if !strings.Contains(body, escapedID) {
// Note: wrapString might break the escaped ID if not careful, but let's check for pieces
t.Logf("Checking for escaped ID: %s", escapedID)
}
escapedName := html.EscapeString(maliciousName)
if !strings.Contains(body, escapedName) {
t.Errorf("Body does not contain escaped Name: %s", escapedName)
}
})

t.Run("PeersPage", func(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, "/peers", nil)
w := httptest.NewRecorder()
server.handlePeers(w, req)
body := w.Body.String()

escapedName := html.EscapeString(maliciousName)
if !strings.Contains(body, escapedName) {
t.Errorf("Body does not contain escaped Name: %s", escapedName)
}
if strings.Contains(body, maliciousURL) && !strings.Contains(body, "href=\"#\"") {
t.Errorf("Malicious URL not sanitized in Peers page")
}
})
}
2 changes: 1 addition & 1 deletion internal/server/templates/grpc.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
</nav>
<div class="container">
{{if .ErrorMsg}}
{{.ErrorMsg}}
<div class="alert alert-danger">{{.ErrorMsg}}</div>
{{end}}

<div class="card shadow-sm mb-4">
Expand Down
Loading
Loading