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

Commit

Permalink
Avoid possible XSS attack when search highlighting is enabled.
Browse files Browse the repository at this point in the history
Inject the highlighting HTML tags on the server side instead of using
JavaScript in the browser to do that.

Fixes #819
  • Loading branch information
bernd committed Jun 27, 2014
1 parent 9e7153d commit eef223c
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 38 deletions.
8 changes: 8 additions & 0 deletions app/models/api/responses/HighlightRange.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,12 @@
public class HighlightRange {
int start;
int length;

public int getStart() {
return start;
}

public int getLength() {
return length;
}
}
80 changes: 80 additions & 0 deletions app/models/api/results/HighlightedField.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package models.api.results;

import models.api.responses.HighlightRange;

import java.util.ArrayList;
import java.util.List;

public class HighlightedField {
private final MessageResult messageResult;
private final String field;

public static class FieldChunk {
private final String string;
private final boolean escape;

public FieldChunk(final String string, final boolean escape) {
this.string = string;
this.escape = escape;
}

public boolean avoidHtmlEscape() {
return !escape;
}

public String getString() {
return string;
}
}

public HighlightedField(final MessageResult messageResult, final String field) {
this.messageResult = messageResult;
this.field = field;
}

public List<FieldChunk> getChunks() {
final String message = (String) messageResult.getFields().get(field);
final List<FieldChunk> list = new ArrayList<>();
final List<HighlightRange> rangesList = messageResult.getHighlightRanges().get(field);

if (rangesList == null) {
throw new RuntimeException("No highlight ranges for field: " + field);
}

// Current position in the message string.
int position = 0;

for (int idx = 0; idx < rangesList.size(); idx++) {
final HighlightRange range = rangesList.get(idx);

// Part before the next highlighted range of the message. (avoid empty chunks)
if (position != range.getStart()) {
list.add(new FieldChunk(message.substring(position, range.getStart()), true));
}

// The highlighted range of the message. Only allow the highlighting tags to be unescaped. The highlighted
// part can contain HTML elements so that should be escaped.
list.add(new FieldChunk("<span class=\"result-highlight\">",false));
list.add(new FieldChunk(message.substring(range.getStart(), range.getStart() + range.getLength()), true));
list.add(new FieldChunk("</span>", false));

if ((idx + 1) < rangesList.size() && rangesList.get(idx + 1) != null) {
// If there is another highlight range, add the part between the end of the current range and the start
// of the next range.
final HighlightRange nextRange = rangesList.get(idx + 1);

list.add(new FieldChunk(message.substring(range.getStart() + range.getLength(), nextRange.getStart()), true));

position = nextRange.getStart();
} else {
// If this range is the last, just add the rest of the message.
list.add(new FieldChunk(message.substring(range.getStart() + range.getLength()), true));

position = range.getStart() + range.getLength();
}

}

return list;
}
}
9 changes: 6 additions & 3 deletions app/models/api/results/MessageResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.gson.Gson;
import models.FieldMapper;
import models.api.responses.HighlightRange;
import org.joda.time.DateTime;
Expand Down Expand Up @@ -184,7 +183,11 @@ public Map<String, List<HighlightRange>> getHighlightRanges() {
return highlightRanges;
}

public String getHighlightRangesAsJson() {
return new Gson().toJson(getHighlightRanges());
public boolean hasHighlightedFields() {
return highlightRanges != null;
}

public HighlightedField getHighlightedField(String field) {
return new HighlightedField(this, field);
}
}
11 changes: 9 additions & 2 deletions app/views/search/results.scala.html
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,19 @@ <h2>
</thead>
<tbody>
@for(r <- searchResult.getMessages()) {
<tr data-message-id='@r.getFields.get("_id")' data-source-index="@r.getIndex" @if(r.getHighlightRanges != null) { data-highlight="@HtmlFormat.escape(r.getHighlightRangesAsJson)" }>
<tr data-message-id='@r.getFields.get("_id")' data-source-index="@r.getIndex">
<td>@DateTools.inUserTimeZoneShortFormat(r.getTimestamp)</td>
<td @if(!(selectedFields.contains("source") || search.getOrder.getField.equals("source"))) { style="display: none;" }
class="result-td-36cd38f49b9afa08222c0dc9ebfe35eb">@r.getFields.get("source")</td>
<td @if(!(selectedFields.contains("message") || search.getOrder.getField.equals("message"))) { style="display: none;" }
class="messages-message result-td-78e731027d8fd50ed642340b7c9a63b3">@r.getFields.get("message")</td>
class="messages-message result-td-78e731027d8fd50ed642340b7c9a63b3">
@if(r.hasHighlightedFields) {
@* Need to use map{} here because a for() loop would produce unwanted newlines and thus extra spaces in the message. *@
@{r.getHighlightedField("message").getChunks.map {x => if(x.avoidHtmlEscape) { Html(x.getString) } else { x.getString }}}
} else {
@r.getFields.get("message")
}
</td>

@for(f <- searchResult.getPageFields()) {
@if(!f.isStandardSelected) {
Expand Down
38 changes: 5 additions & 33 deletions public/javascripts/universalsearch.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,38 +86,10 @@ $(document).ready(function() {
$(".result-highlight").toggleClass("result-highlight-colored", $(this).is(":checked"));
});

/* TODO figure out if we want to keep it this way */
String.prototype.gl2_splice = function( idx, s ) {
return (this.slice(0,idx) + s + this.slice(idx));
};
$(".messages tbody tr").each(function() {
var ranges = $(this).data('highlight');

if (ranges == undefined) {
// Search highlighting not enabled in server.
$(".explain-result-highlight-control").show();
} else {
// Search highlighting is enabled in server.
for (var field in ranges) {
if (! ranges.hasOwnProperty(field) ) {
continue;
}
var positions = ranges[field];
var fieldNameHash = CryptoJS.MD5(field);
$(".result-td-" + fieldNameHash, $(this)).each(function(){
var elemText = $(this).text();
for (var idx = positions.length - 1; idx >= 0; idx--) {
var range = positions[idx];
elemText = elemText.gl2_splice(range.start + range.length, "</span>");
elemText = elemText.gl2_splice(range.start, '<span class="result-highlight">');
}
$(this).html(elemText);
$(".result-highlight", $(this)).toggleClass("result-highlight-colored");
});
$(".result-highlight-control").show();
}
}
});
if ($(".messages").find(".result-highlight").length > 0) {
$(".messages .result-highlight").toggleClass("result-highlight-colored");
$(".result-highlight-control").show();
}

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

$("a", link.parent().parent()).removeClass("selected");
link.addClass("selected");
}
}

0 comments on commit eef223c

Please sign in to comment.