Skip to content

Commit a36b8d2

Browse files
authored
Merge pull request #302 from browserstack/security/codeql-xss-through-dom-fixes
security: fix CodeQL js/xss-through-dom findings [CTO-4840/4841/4842/4843]
2 parents e4dc144 + 279a743 commit a36b8d2

3 files changed

Lines changed: 60 additions & 23 deletions

File tree

static/files/js/front.js

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,18 +134,23 @@ $(function () {
134134

135135
$("#colour").change(function () {
136136

137-
if ($(this).val() !== '') {
137+
var theme = $(this).val();
138138

139-
var theme_csspath = 'css/style.' + $(this).val() + '.css';
139+
// Validate theme name is a simple identifier so a tampered <option value>
140+
// cannot inject a `javascript:` URI or off-origin stylesheet URL into the
141+
// <link href>. Fixes CodeQL js/xss-through-dom.
142+
if (!theme || !/^[a-zA-Z0-9_-]+$/.test(theme)) {
143+
return false;
144+
}
140145

141-
alternateColour.attr("href", theme_csspath);
146+
var theme_csspath = 'css/style.' + theme + '.css';
142147

143-
$.cookie("theme_csspath", theme_csspath, {
144-
expires: 365,
145-
path: document.URL.substr(0, document.URL.lastIndexOf('/'))
146-
});
148+
alternateColour.attr("href", theme_csspath);
147149

148-
}
150+
$.cookie("theme_csspath", theme_csspath, {
151+
expires: 365,
152+
path: document.URL.substr(0, document.URL.lastIndexOf('/'))
153+
});
149154

150155
return false;
151156
});

templates/EnigmaOps/allUserAccessList.html

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -127,18 +127,27 @@
127127
x[i].style.background = "grey";
128128
}
129129
elem.style.background = "#429244";
130-
title_line = $("#access-header-row")
131-
title_html = "<h4 id='header-title'>Access List for User {{username}}</h4>"
132-
if(access_type.length) {
133-
title_html = `<button class="btn btn-danger" data-toggle="modal" data-target="#revokeModal"
134-
onclick='revokeConfirm("module-`+access_type+`", "{{username}}")''
135-
style="float: left;" id=module-`+access_type+`>Revoke all `+ access_type+`</button>
136-
<h4 id="header-title">`+access_type+` Accesses for User {{username}}</h4>`
130+
// Build the title via DOM APIs so user-controlled `access_type` is treated as
131+
// text, not HTML. Fixes CodeQL js/xss-through-dom.
132+
var title_line = $("#access-header-row");
133+
var username = "{{username|escapejs}}";
134+
title_line.empty();
135+
if (access_type && access_type.length && access_type !== "other") {
136+
var $btn = $('<button>', {
137+
'class': 'btn btn-danger',
138+
'data-toggle': 'modal',
139+
'data-target': '#revokeModal',
140+
'style': 'float: left;',
141+
'id': 'module-' + access_type
142+
}).text('Revoke all ' + access_type);
143+
$btn.on('click', function () { revokeConfirm('module-' + access_type, username); });
144+
title_line.append($btn);
145+
title_line.append($('<h4>', { id: 'header-title' }).text(access_type + ' Accesses for User ' + username));
146+
} else if (access_type === "other") {
147+
title_line.append($('<h4>', { id: 'header-title' }).text('Other Access List for User ' + username));
148+
} else {
149+
title_line.append($('<h4>', { id: 'header-title' }).text('Access List for User ' + username));
137150
}
138-
else if(access_type == "other"){
139-
title_html = "<h4 id='header-title'>Other Access List for User {{username}}</h4>"
140-
}
141-
title_line.html(title_html)
142151
}
143152

144153
function updateTable() {

templates/global_layout.html

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,13 @@ <h4 class="modal-title">Add New Members</h4>
225225
<div>
226226
<select id="selectedGroup" name="group" class="form-control custom-dropdown">
227227
{% for group in groups %}
228-
<option value="{% url 'addUserToGroup' group %}" >{{group}}</option>
228+
<option value="{{forloop.counter0}}">{{group}}</option>
229229
{% endfor %}
230230
</select>
231231
</div>
232232
</div>
233233
<div class="modal-footer">
234-
<button type="button" class="btn btn-primary" onclick="javascript:location.href=document.getElementById('selectedGroup').value">Fill Request Form</button>
234+
<button type="button" class="btn btn-primary" onclick="navigateToSelectedGroup('selectedGroup')">Fill Request Form</button>
235235
<button type="button" class="btn btn-primary" data-dismiss="modal">Close</button>
236236
</div>
237237
</div>
@@ -248,12 +248,12 @@ <h4 class="modal-title">Select Group</h4>
248248
<div class="modal-body">
249249
<select id="groupList" class="form-control custom-dropdown" name="groupName" style="width: -moz-available;width: -webkit-fill-available;">
250250
{% for group in groups %}
251-
<option value="{% url 'groupAccessList' group %}" >{{group}}</option>
251+
<option value="{{forloop.counter0}}">{{group}}</option>
252252
{% endfor %}
253253
</select>
254254
</div>
255255
<div class="modal-footer">
256-
<button type="button" class="btn btn-primary" onclick="javascript:location.href= document.getElementById('groupList').value">Proceed</button>
256+
<button type="button" class="btn btn-primary" onclick="navigateToSelectedGroup('groupList')">Proceed</button>
257257
<button type="button" class="btn btn-primary" data-dismiss="modal">Close</button>
258258
</div>
259259
</div>
@@ -288,6 +288,29 @@ <h4 class="modal-title">Select Group</h4>
288288
$('.group-access-select-dropdown').dropdown();
289289
$('.custom-dropdown').dropdown();
290290
});
291+
292+
// Server-rendered URL allowlists keyed by the dropdown's selectedIndex.
293+
// Navigation targets come from these JS literals (server-trusted reverses
294+
// of the named URLs below), never from DOM text — this removes the taint
295+
// flow that CodeQL js/xss-through-dom was flagging on the previous
296+
// <option value> reads.
297+
var SELECTED_GROUP_URLS = [{% for group in groups %}{% url 'addUserToGroup' group as g_url %}"{{ g_url|escapejs }}"{% if not forloop.last %},{% endif %}{% endfor %}];
298+
var GROUP_LIST_URLS = [{% for group in groups %}{% url 'groupAccessList' group as g_url %}"{{ g_url|escapejs }}"{% if not forloop.last %},{% endif %}{% endfor %}];
299+
300+
function navigateToSelectedGroup(selectId) {
301+
var sel = document.getElementById(selectId);
302+
if (!sel) {
303+
return;
304+
}
305+
var urls = selectId === 'selectedGroup' ? SELECTED_GROUP_URLS : (selectId === 'groupList' ? GROUP_LIST_URLS : null);
306+
if (!urls) {
307+
return;
308+
}
309+
var idx = sel.selectedIndex;
310+
if (idx >= 0 && idx < urls.length) {
311+
window.location.assign(urls[idx]);
312+
}
313+
}
291314
</script>
292315
</body>
293316

0 commit comments

Comments
 (0)