Skip to content

Commit

Permalink
Fix issues with inconsistency in input sanitisation leading to XSS ve…
Browse files Browse the repository at this point in the history 
…ctors
  • Loading branch information
@chris-allan @jburel
chris-allan authored and jburel committed Oct 7, 2021
1 parent 60bcdd9 commit 0168067
Show file tree
Hide file tree
Showing 26 changed files with 156 additions and 162 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@

5.11.0 (October 2021)
---------------------

- Security vulnerability fixes for
[2021-SV3](https://www.openmicroscopy.org/security/advisories/2021-SV3)

5.11.0.rc1 (September 2021)
---------------------------

Expand Down
3 changes: 2 additions & 1 deletion omeroweb/webadmin/templates/webadmin/includes/drivespaceStats.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
for(i=0; i<chart_data.length; i++) {
rowData = chart_data[i];
rowId = typeof rowData.userId != 'undefined' ? rowData.userId : rowData.groupId;
tableRows.push('<tr><td class="link">' + rowData.label + '(id:'+ rowId +')</td>');
tableRows.push('<tr><td class="link">' + rowData.label.escapeHTML() + '(id:'+ rowId +')</td>');
tableRows.push('<td class="link">' + rowData.data.filesizeformat() + '</td></tr>');
}

Expand Down Expand Up @@ -50,6 +50,7 @@

for(i=0; i<data.length; i++) {
var slice = data[i];
slice.label = slice.label.escapeHTML();
if(i === MAX_SLICES){
chart_data.push({label:'Others', data:slice.data});
} else if (i > MAX_SLICES) {
Expand Down
6 changes: 3 additions & 3 deletions omeroweb/webclient/static/webclient/javascript/ome.chgrp.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ $(function() {
data_owners = data.owners; // save for later
var ownernames = [];
for (var o=0; o<data.owners.length; o++) {ownernames.push(data.owners[o][1]);}
var headerTxt = "<p>Move data owned by " + ownernames.join(", ") + ".</p>" +
var headerTxt = "<p>Move data owned by " + ownernames.join(", ").escapeHTML() + ".</p>" +
"<h1>Please choose target group below:</h1>";
$group_chooser.append(headerTxt);

Expand All @@ -72,7 +72,7 @@ $(function() {
var g = data.groups[i];
html += "<div class='chgrpGroup' data-gid='"+ g.id + "'>";
html += "<img src='" + permsIcon(g.perms) + "'/>";
html += g.name + "<hr></div>";
html += g.name.escapeHTML() + "<hr></div>";
}
// If no target groups found...
if (data.groups.length === 0) {
Expand Down Expand Up @@ -407,7 +407,7 @@ $(function() {
if (count === 1) otype = otype.slice(0, -1); // remove s
var namesList = [], names;
unlinked.forEach(function (u) {
namesList.push(u.name);
namesList.push(u.name.escapeHTML());
});
names = namesList.join(", ");
names = " <i title='" + namesList.join("\n") + "'>(" + names.slice(0, 40) + (names.length > 40 ? "..." : "") + ")</i>";
Expand Down
4 changes: 2 additions & 2 deletions omeroweb/webclient/static/webclient/javascript/ome.chown.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ $(function() {
var templateText = `
<!-- Hidden fields for objects. e.g. name='Image' value='1,2,3' -->
<% _.each(selobjs, function(obj, idx) { %>
<input name='<%= obj.split("=")[0] %>' value='<%= obj.split("=")[1] %>' hidden/>
<input name='<%- obj.split("=")[0] %>' value='<%- obj.split("=")[1] %>' hidden/>
<% }) %>
<!-- List target new owners -->
Expand All @@ -50,7 +50,7 @@ $(function() {
<% _.each(exps, function(exp, idx) { %>
<label>
<input name='owner_id' type='radio' value='<%= exp['@id'] %>'/>
<%= exp.FirstName%> <%= exp.LastName %>
<%- exp.FirstName%> <%- exp.LastName %>
</label>
<br/>
<% }) %>
Expand Down
1 change: 0 additions & 1 deletion omeroweb/webclient/static/webclient/javascript/ome.right_panel_comments_pane.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ var CommentsPane = function CommentsPane($element, opts) {
ann.link.owner = experimenters[ann.link.owner.id];
}
ann.addedBy = [ann.link.owner.id];
ann.textValue = _.escape(ann.textValue);
return ann;
});

Expand Down
2 changes: 1 addition & 1 deletion omeroweb/webclient/static/webclient/javascript/ome.right_panel_customanns_pane.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ var CustomAnnsPane = function CustomAnnsPane($element, opts) {
var attrs = ['textValue', 'timeValue', 'termValue', 'longValue', 'doubleValue', 'boolValue'];
attrs.forEach(function(a){
if (ann[a] !== undefined){
ann.value = _.escape(ann[a]);
ann.value = ann[a];
}
});
if (objects.length > 1) {
Expand Down
1 change: 0 additions & 1 deletion omeroweb/webclient/static/webclient/javascript/ome.right_panel_fileanns_pane.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ var FileAnnsPane = function FileAnnsPane($element, opts) {
}
// AddedBy IDs for filtering
ann.addedBy = [ann.link.owner.id];
ann.description = _.escape(ann.description);
ann.file.size = ann.file.size !== null ? ann.file.size.filesizeformat() : "";
return ann;
});
Expand Down
6 changes: 3 additions & 3 deletions omeroweb/webclient/static/webclient/javascript/ome.right_panel_tags_pane.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ var TagPane = function TagPane($element, opts) {
}
// AddedBy IDs for filtering
tag.addedBy = [tag.link.owner.id];
tag.textValue = _.escape(tag.textValue);
tag.description = _.escape(tag.description);
tag.textValue = tag.textValue;
tag.description = tag.description;
tag.canRemove = tag.link.permissions.canDelete;
return tag;
});
Expand All @@ -158,7 +158,7 @@ var TagPane = function TagPane($element, opts) {
var tagId = tag.id,
linkOwner = tag.link.owner.id;
if (summary[tagId] === undefined) {
summary[tagId] = {'textValue': tag.textValue,
summary[tagId] = {'textValue': _.escape(tag.textValue),
'id': tag.id,
'canRemove': false,
'canRemoveCount': 0,
Expand Down
60 changes: 23 additions & 37 deletions omeroweb/webclient/static/webclient/javascript/ome.tagging_form.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,9 @@ var tagging_form = function(
for (var id in all_tags) {
var tag = all_tags[id];
if (tag && !(id in child_tags)) {
html += create_tag_html(tag.t, tag.d, owners[tag.o], tag.i,
null, tag.s !== 0);
html += create_tag_html(
tag.t, tag.d, tag.i, null, tag.s !== 0
);
tag.sort_key = tag.t.toLowerCase();
if (tag.s) {
for (var sid in tag.s) {
Expand All @@ -108,8 +109,8 @@ var tagging_form = function(
child.sort_key = (tag.t.toLowerCase() +
child.t.toLowerCase());
html += create_tag_html(
child.t, child.d, owners[child.o], child.i,
tag.i);
child.t, child.d, child.i, tag.i
);
}
}
}
Expand All @@ -118,15 +119,7 @@ var tagging_form = function(
div_all_tags.append(html);
// TODO This tooltip application is used until the extra data has loaded
// at which point the tooltips are updated and this handler is replaced?
$(".tag_selection div").on('click', tag_click).tooltip({
track: false,
show: false,
hide: false,
items: '[data-content]',
content: function() {
return $(this).data('content');
}
});
$(".tag_selection div").on('click', tag_click);
};

var update_selected_labels = function() {
Expand All @@ -145,7 +138,7 @@ var tagging_form = function(
if (tagset) {
$("#id_selected_tag_set").html(
"Add a new tag in <span class='tagset-title'>" +
tagset.text() + "</span> tag set and select it immediately:");
tagset.text().escapeHTML() + "</span> tag set and select it immediately:");
} else {
$("#id_selected_tag_set").text(
"Add a new tag and select it immediately:");
Expand All @@ -169,6 +162,7 @@ var tagging_form = function(
track: false,
show: false,
hide: false,
items: '[data-id]', // Just needs an attribute that exists
content: title
});
};
Expand Down Expand Up @@ -369,45 +363,35 @@ var tagging_form = function(
}
};

var encode_html = function(text) {
return $('<div/>').text(text).html();
};

var create_tag_title = function(description, owner, tagset, link_owner) {
var title = "";
if (owner) {
title += "<b>Owner:</b> " + owner + "<br />";
title += "<b>Owner:</b> " + owner.escapeHTML() + "<br />";
}
if (link_owner) {
title += "<b>Linked by:</b> " + link_owner + "<br />";
title += "<b>Linked by:</b> " + link_owner.escapeHTML() + "<br />";
}
if (description) {
title += "<b>Description:</b> " + description + "<br />";
title += "<b>Description:</b> " + description.escapeHTML() + "<br />";
}
if (tagset) {
title += "<b>Tag set:</b> " + tagset + "<br />";
title += "<b>Tag set:</b> " + tagset.escapeHTML() + "<br />";
}
return title;
};

var create_tag_html = function(text, description, owner, id, parent_id,
var create_tag_html = function(text, description, id, parent_id,
is_tagset) {
var cls = is_tagset ? "alltags-tagset" :
(parent_id ? "alltags-childtag" : "alltags-tag");
var html = "<div class='" + cls + "' data-id='" + id + "'";
var tagset;
if (parent_id) {
html += " data-set='" + parent_id + "'";
tagset = all_tags[parent_id].t;
}
if (id < 0) { // new tag, save description
html += " data-description='" + encode_html(description) + "'";
html += " data-description='" + description.escapeHTML() + "'";
}
var title = create_tag_title(description, owner, tagset);
// Add content even if it is empty
// and handle case for no tooltip in the tooltip code
html += " data-content='" + title.replace(/'/g, "&#39;") + "'";
html += ">" + encode_html(text) + "</div>";
html += ">" + text.escapeHTML() + "</div>";
return html;
};

Expand Down Expand Up @@ -633,7 +617,7 @@ var tagging_form = function(
new_tag_counter -= 1;
var tagset_id = (tagset ? parseInt(tagset.attr('data-id'), 10) :
false);
owners[me] = owners[me] || my_name;
owners[me] = owners[me] || _.unescape(my_name);
all_tags[new_tag_counter] = {
i: new_tag_counter,
d: description,
Expand All @@ -644,16 +628,18 @@ var tagging_form = function(
'') + text.toLowerCase()
};
var div = $(create_tag_html(
text, description, my_name, new_tag_counter,
text, description, new_tag_counter,
tagset ? tagset.attr('data-id') : null));
var title = create_tag_title(
description, _.unescape(my_name),
tagset_id? all_tags[tagset_id].t : null
);
div.addClass('ui-selected').on('click', tag_click).tooltip({
track: true,
show: false,
hide: false,
items: '[data-content]',
content: function() {
return $(this).data('content');
}
items: '[data-id]', // Just needs an attribute that exists
content: title
});
$("div.ui-selected", div_selected_tags).removeClass('ui-selected');
div_selected_tags.append(div);
Expand Down
4 changes: 2 additions & 2 deletions omeroweb/webclient/static/webclient/javascript/ome.thumbnail_figure.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ $(document).ready(function() {
// start a new container...
topLevelTag = tagData.tags[0] || tagsText; // Group under 1st tag (unless we have no tags)

$tr = $('<tr><th><h2>' + topLevelTag + '</h2></th></tr>');
$tr = $('<tr><th><h2>' + topLevelTag.escapeHTML() + '</h2></th></tr>');
$tr.appendTo($this);
$td = $('<td></td>').appendTo($tr);
}
Expand Down Expand Up @@ -263,4 +263,4 @@ $(document).ready(function() {
}
});

});
});
26 changes: 13 additions & 13 deletions omeroweb/webclient/templates/webclient/annotations/comments_underscore.html
View file
Original file line number Diff line number Diff line change
@@ -1,36 +1,36 @@
<% _.each(anns, function(ann) { %>
<div class="ann_comment_wrapper" data-added-by="<% print (ann.addedBy.join(',')) %>">
<div class="ann_comment_wrapper" data-added-by="<% print(_.escape(ann.addedBy.join(','))) %>">

<div class="avatar">
<img src="<%= webindex %>avatar/<%= ann.owner.id %>/"
alt="<%= ann.owner.firstName %> <%= ann.owner.lastName %>"
title="<%= ann.owner.firstName %> <%= ann.owner.lastName %>" />
<img src="<%= webindex %>avatar/<%- ann.owner.id %>/"
alt="<%- ann.owner.firstName %> <%- ann.owner.lastName %>"
title="<%- ann.owner.firstName %> <%- ann.owner.lastName %>" />
</div>

<div class="ann_comment_text tooltip">
<div class="ann_comment_header">
<strong>
<%= ann.owner.firstName %> <%= ann.owner.lastName %>
</strong>
<%- ann.owner.firstName %> <%- ann.owner.lastName %>
</strong>
at
<% print(OME.formatDate(ann.link.date)) %>
</div>

<% if (ann.permissions.canDelete) { %>
<img class='removeComment' id="<%= ann.id %>-comment"
<img class='removeComment' id="<%- ann.id %>-comment"
src="<%= static %>image/icon_basic_delete.png"
url='<%= webindex %>action/remove/comment/<%= ann.id %>/'
url='<%= webindex %>action/remove/comment/<%- ann.id %>/'
title="Delete comment"/>
<% } %>

<div class='commentText'><%= ann.textValue %></div>
<div class='commentText'><%- ann.textValue %></div>
</div>

<% if (ann.ns || ann.description) { %>
<span class="tooltip_html" style='display:none'>
<b>ID:</b> <%= ann.id %><br />
<% if (ann.ns) { %><b>Namespace:</b> <%= ann.ns %><br /><% } %>
<% if (ann.description) { %><b>Description:</b> <%= ann.description %><br /><% } %>
<b>ID:</b> <%- ann.id %><br />
<% if (ann.ns) { %><b>Namespace:</b> <%- ann.ns %><br /><% } %>
<% if (ann.description) { %><b>Description:</b> <%- ann.description %><br /><% } %>
</span>
<% } %>

Expand Down
24 changes: 12 additions & 12 deletions omeroweb/webclient/templates/webclient/annotations/customanns_underscore.html
View file
Original file line number Diff line number Diff line change
@@ -1,31 +1,31 @@

<% _.each(anns, function(ann) { %>

<tr data-added-by="<% print (ann.addedBy.join(',')) %>">
<th><%= ann.type %></th>
<tr data-added-by="<% print(_.escape(ann.addedBy.join(','))) %>">
<th><%- ann.type %></th>
<td>
<div class="tooltip">
<% if (ann.type === 'Xml') { %>
<div><% print((ann.value + "").slice(0, 20)) %>...</div>
<div><% print(_.escape((ann.value + "").slice(0, 20))) %>...</div>
<div class="show_xml">Open in new window</div>
<div style="display: none"><%= ann.value %></div>
<div style="display: none"><%- ann.value %></div>
<% } else { %>
<div><%= ann.value %></div>
<div><%- ann.value %></div>
<% } %>
</div>

<span class="tooltip_html" style='display:none'>
<b>ID:</b> <%= ann.id %><br />
<% if (ann.ns) { %><b>Namespace:</b> <%= ann.ns %><br /><% } %>
<% if (ann.description) { %><b>Description:</b> <%= ann.description %><br /><% } %>
<b>Owner:</b> <%= ann.owner.firstName %> <%= ann.owner.lastName %><br />
<b>ID:</b> <%- ann.id %><br />
<% if (ann.ns) { %><b>Namespace:</b> <%- ann.ns %><br /><% } %>
<% if (ann.description) { %><b>Description:</b> <%- ann.description %><br /><% } %>
<b>Owner:</b> <%- ann.owner.firstName %> <%- ann.owner.lastName %><br />
<b>Date:</b> <% print(OME.formatDate(ann.date)) %>

<% if (ann.parent) { %>
<br />
<b>Linked to:</b> <%= ann.parent.class %> <%= ann.parent.id %>
<b>Linked to:</b> <%- ann.parent.class %> <%- ann.parent.id %>
<% } %>
</span>
</td>
</tr>
<% }) %>
<% }) %>
Loading

0 comments on commit 0168067

Please sign in to comment.