Skip to content
This repository was archived by the owner on Jan 8, 2019. It is now read-only.

Commit eef223c

Browse files
committed
Avoid possible XSS attack when search highlighting is enabled.
Inject the highlighting HTML tags on the server side instead of using JavaScript in the browser to do that. Fixes #819
1 parent 9e7153d commit eef223c

File tree

5 files changed

+108
-38
lines changed

5 files changed

+108
-38
lines changed

app/models/api/responses/HighlightRange.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,12 @@
2121
public class HighlightRange {
2222
int start;
2323
int length;
24+
25+
public int getStart() {
26+
return start;
27+
}
28+
29+
public int getLength() {
30+
return length;
31+
}
2432
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
package models.api.results;
2+
3+
import models.api.responses.HighlightRange;
4+
5+
import java.util.ArrayList;
6+
import java.util.List;
7+
8+
public class HighlightedField {
9+
private final MessageResult messageResult;
10+
private final String field;
11+
12+
public static class FieldChunk {
13+
private final String string;
14+
private final boolean escape;
15+
16+
public FieldChunk(final String string, final boolean escape) {
17+
this.string = string;
18+
this.escape = escape;
19+
}
20+
21+
public boolean avoidHtmlEscape() {
22+
return !escape;
23+
}
24+
25+
public String getString() {
26+
return string;
27+
}
28+
}
29+
30+
public HighlightedField(final MessageResult messageResult, final String field) {
31+
this.messageResult = messageResult;
32+
this.field = field;
33+
}
34+
35+
public List<FieldChunk> getChunks() {
36+
final String message = (String) messageResult.getFields().get(field);
37+
final List<FieldChunk> list = new ArrayList<>();
38+
final List<HighlightRange> rangesList = messageResult.getHighlightRanges().get(field);
39+
40+
if (rangesList == null) {
41+
throw new RuntimeException("No highlight ranges for field: " + field);
42+
}
43+
44+
// Current position in the message string.
45+
int position = 0;
46+
47+
for (int idx = 0; idx < rangesList.size(); idx++) {
48+
final HighlightRange range = rangesList.get(idx);
49+
50+
// Part before the next highlighted range of the message. (avoid empty chunks)
51+
if (position != range.getStart()) {
52+
list.add(new FieldChunk(message.substring(position, range.getStart()), true));
53+
}
54+
55+
// The highlighted range of the message. Only allow the highlighting tags to be unescaped. The highlighted
56+
// part can contain HTML elements so that should be escaped.
57+
list.add(new FieldChunk("<span class=\"result-highlight\">",false));
58+
list.add(new FieldChunk(message.substring(range.getStart(), range.getStart() + range.getLength()), true));
59+
list.add(new FieldChunk("</span>", false));
60+
61+
if ((idx + 1) < rangesList.size() && rangesList.get(idx + 1) != null) {
62+
// If there is another highlight range, add the part between the end of the current range and the start
63+
// of the next range.
64+
final HighlightRange nextRange = rangesList.get(idx + 1);
65+
66+
list.add(new FieldChunk(message.substring(range.getStart() + range.getLength(), nextRange.getStart()), true));
67+
68+
position = nextRange.getStart();
69+
} else {
70+
// If this range is the last, just add the rest of the message.
71+
list.add(new FieldChunk(message.substring(range.getStart() + range.getLength()), true));
72+
73+
position = range.getStart() + range.getLength();
74+
}
75+
76+
}
77+
78+
return list;
79+
}
80+
}

app/models/api/results/MessageResult.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import com.google.common.collect.ImmutableSet;
2424
import com.google.common.collect.Lists;
2525
import com.google.common.collect.Maps;
26-
import com.google.gson.Gson;
2726
import models.FieldMapper;
2827
import models.api.responses.HighlightRange;
2928
import org.joda.time.DateTime;
@@ -184,7 +183,11 @@ public Map<String, List<HighlightRange>> getHighlightRanges() {
184183
return highlightRanges;
185184
}
186185

187-
public String getHighlightRangesAsJson() {
188-
return new Gson().toJson(getHighlightRanges());
186+
public boolean hasHighlightedFields() {
187+
return highlightRanges != null;
188+
}
189+
190+
public HighlightedField getHighlightedField(String field) {
191+
return new HighlightedField(this, field);
189192
}
190193
}

app/views/search/results.scala.html

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -371,12 +371,19 @@ <h2>
371371
</thead>
372372
<tbody>
373373
@for(r <- searchResult.getMessages()) {
374-
<tr data-message-id='@r.getFields.get("_id")' data-source-index="@r.getIndex" @if(r.getHighlightRanges != null) { data-highlight="@HtmlFormat.escape(r.getHighlightRangesAsJson)" }>
374+
<tr data-message-id='@r.getFields.get("_id")' data-source-index="@r.getIndex">
375375
<td>@DateTools.inUserTimeZoneShortFormat(r.getTimestamp)</td>
376376
<td @if(!(selectedFields.contains("source") || search.getOrder.getField.equals("source"))) { style="display: none;" }
377377
class="result-td-36cd38f49b9afa08222c0dc9ebfe35eb">@r.getFields.get("source")</td>
378378
<td @if(!(selectedFields.contains("message") || search.getOrder.getField.equals("message"))) { style="display: none;" }
379-
class="messages-message result-td-78e731027d8fd50ed642340b7c9a63b3">@r.getFields.get("message")</td>
379+
class="messages-message result-td-78e731027d8fd50ed642340b7c9a63b3">
380+
@if(r.hasHighlightedFields) {
381+
@* Need to use map{} here because a for() loop would produce unwanted newlines and thus extra spaces in the message. *@
382+
@{r.getHighlightedField("message").getChunks.map {x => if(x.avoidHtmlEscape) { Html(x.getString) } else { x.getString }}}
383+
} else {
384+
@r.getFields.get("message")
385+
}
386+
</td>
380387

381388
@for(f <- searchResult.getPageFields()) {
382389
@if(!f.isStandardSelected) {

public/javascripts/universalsearch.js

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -86,38 +86,10 @@ $(document).ready(function() {
8686
$(".result-highlight").toggleClass("result-highlight-colored", $(this).is(":checked"));
8787
});
8888

89-
/* TODO figure out if we want to keep it this way */
90-
String.prototype.gl2_splice = function( idx, s ) {
91-
return (this.slice(0,idx) + s + this.slice(idx));
92-
};
93-
$(".messages tbody tr").each(function() {
94-
var ranges = $(this).data('highlight');
95-
96-
if (ranges == undefined) {
97-
// Search highlighting not enabled in server.
98-
$(".explain-result-highlight-control").show();
99-
} else {
100-
// Search highlighting is enabled in server.
101-
for (var field in ranges) {
102-
if (! ranges.hasOwnProperty(field) ) {
103-
continue;
104-
}
105-
var positions = ranges[field];
106-
var fieldNameHash = CryptoJS.MD5(field);
107-
$(".result-td-" + fieldNameHash, $(this)).each(function(){
108-
var elemText = $(this).text();
109-
for (var idx = positions.length - 1; idx >= 0; idx--) {
110-
var range = positions[idx];
111-
elemText = elemText.gl2_splice(range.start + range.length, "</span>");
112-
elemText = elemText.gl2_splice(range.start, '<span class="result-highlight">');
113-
}
114-
$(this).html(elemText);
115-
$(".result-highlight", $(this)).toggleClass("result-highlight-colored");
116-
});
117-
$(".result-highlight-control").show();
118-
}
119-
}
120-
});
89+
if ($(".messages").find(".result-highlight").length > 0) {
90+
$(".messages .result-highlight").toggleClass("result-highlight-colored");
91+
$(".result-highlight-control").show();
92+
}
12193

12294
// Save a search: Open save dialog.
12395
$(".save-search").on("click", function(e) {
@@ -235,4 +207,4 @@ function activateTimerangeChooser(selectorName, link) {
235207

236208
$("a", link.parent().parent()).removeClass("selected");
237209
link.addClass("selected");
238-
}
210+
}

0 commit comments

Comments
 (0)