Skip to content

Commit 883b702

Browse files
vedharishvedharish
authored
Merge pull request #320 from browserstack/security/enigma-dom-xss
security(xss): DOM-safe rendering in access list + pending requests (F-024/F-025)
2 parents 9cbfae5 + f2fdd8b commit 883b702

2 files changed

Lines changed: 70 additions & 34 deletions

File tree

‎templates/EnigmaOps/allUserAccessList.html‎

Lines changed: 57 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -156,40 +156,69 @@
156156
urlBuilder = buildUrlFromParams("json");
157157
$.ajax({url: urlBuilder,
158158
success: function(result){
159-
html_string = "No Results"
160-
for(i=0;i<result["dataList"].length;i++){
161-
record = result["dataList"][i]
162-
html_string += "<tr>"
163-
html_string += "<td>"+record["user"]+"</td>"
164-
html_string += "<td>"+record["access_desc"]+"</td>"
165-
html_string += "<td>"
166-
for(j=0;j<record["access_label"].length;j++)
167-
html_string += record["access_label"][j]+"<br>"
168-
html_string += "</td>"
159+
// F-024: build rows with DOM APIs + .text() so DB-stored, user-controlled
160+
// fields (requestId/user/approver/...) are treated as text, never HTML.
161+
var isOps = {% if is_ops %}true{% else %}false{% endif %};
162+
var $tbody = $("#tableData");
163+
$tbody.empty();
164+
var dataList = result["dataList"] || [];
165+
if (dataList.length === 0) {
166+
$tbody.text("No Results");
167+
}
168+
for(var i=0;i<dataList.length;i++){
169+
var record = dataList[i]
170+
var $tr = $("<tr>");
171+
$tr.append($("<td>").text(record["user"]));
172+
$tr.append($("<td>").text(record["access_desc"]));
173+
var $labelTd = $("<td>");
174+
for(var j=0;j<record["access_label"].length;j++){
175+
$labelTd.append(document.createTextNode(record["access_label"][j]));
176+
$labelTd.append($("<br>"));
177+
}
178+
$tr.append($labelTd);
179+
var $statusTd = $("<td>").attr("id", record["requestId"]+"-access-status");
180+
if(record["status"] != "Revoked"){
181+
$statusTd.text(record["status"]);
182+
} else {
183+
$statusTd.append(document.createTextNode(record["status"]));
184+
$statusTd.append($("<br>"));
185+
$statusTd.append(document.createTextNode("By-"+record["revoker"]));
186+
}
187+
$tr.append($statusTd);
188+
var $btnTd = $("<td>").attr("id", record["requestId"]+"-revoke-button");
169189
if(record["status"] != "Revoked"){
170-
html_string += "<td id='"+record["requestId"]+"-access-status'>"+record["status"]+"</td>"
190+
var $btn = $("<button>", {
191+
"class": "btn btn-danger",
192+
"data-toggle": "modal",
193+
"data-target": "#revokeModal",
194+
"id": record["requestId"]
195+
}).text("Mark Revoked");
196+
// read the request id off the element — no inline onclick/JS-context injection
197+
$btn.on("click", function(){ revokeConfirm($(this).attr("id")); });
198+
$btnTd.append($btn);
171199
}
172-
else{
173-
html_string += "<td id='"+record["requestId"]+"-access-status'>"+record["status"]+"<br>By-"+record["revoker"]+"</td>"
200+
$tr.append($btnTd);
201+
$tr.append($("<td>").text(record["requested_on"]));
202+
$tr.append($("<td>").text(record["approver"]));
203+
$tr.append($("<td>").text(record["updated_on"]));
204+
$tr.append($("<td>").text(record["offboarding_date"]));
205+
$tr.append($("<td>").text(record["grantOwner"]));
206+
$tr.append($("<td>").text(record["revokeOwner"]));
207+
var $regrantTd = $("<td>");
208+
if(isOps){
209+
$regrantTd.append($("<a>", {
210+
"class": "btn btn-primary",
211+
"target": "_blank",
212+
"href": "/individual_resolve?requestId="+encodeURIComponent(record["requestId"])+"&ops_resolve=true"
213+
}).text("ReGrant"));
174214
}
175-
html_string += "<td id='"+record["requestId"]+"-revoke-button'>"
176-
if(record["status"] != "Revoked")
177-
html_string += '<button id="'+record["requestId"]+`" class="btn btn-danger" data-toggle="modal" data-target="#revokeModal" onclick='revokeConfirm("`+record["requestId"]+`")'>Mark Revoked</button>`
178-
html_string += "</td>"
179-
html_string += "<td>"+record["requested_on"]+"</td>"
180-
html_string += "<td>"+record["approver"]+"</td>"
181-
html_string += "<td>"+record["updated_on"]+"</td>"
182-
html_string += "<td>"+record["offboarding_date"]+"</td>"
183-
html_string += "<td>"+record["grantOwner"]+"</td>"
184-
html_string += "<td>"+record["revokeOwner"]+"</td>"
185-
html_string += "<td>{% if is_ops %} <a class=\"btn btn-primary\" target=\"_blank\" href=\"/individual_resolve?requestId="+encodeURIComponent(record["requestId"])+"&ops_resolve=true\">ReGrant</a></td>{% endif %}"
186-
html_string += "<td>"+record["type"]+"</td>"
187-
html_string += "</tr>"
215+
$tr.append($regrantTd);
216+
$tr.append($("<td>").text(record["type"]));
217+
$tbody.append($tr);
188218
}
189-
$("#tableData").html(html_string);
190219
page_number = result['current_page'];
191220

192-
$("#pagedisplay").html("Page "+String(result['current_page'])+" of "+String(result['last_page']))
221+
$("#pagedisplay").text("Page "+String(result['current_page'])+" of "+String(result['last_page']))
193222
},
194223
error: function(result){
195224
alert("Error occured while fetching data")

‎templates/EnigmaOps/pendingRequests.html‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,15 @@
173173

174174
$(".dropdown-menu a:first-child").trigger('click')
175175

176+
// F-025: server-supplied msg/error must be inserted as text, not HTML.
177+
function setColoredMessage(el, text, color) {
178+
if (!el) return;
179+
el.innerHTML = "";
180+
var p = document.createElement("p");
181+
p.style.color = color;
182+
p.textContent = text;
183+
el.appendChild(p);
184+
}
176185
function updateStatus(result, selector) {
177186
for (var request_id in result["response"]) {
178187
div_id = request_id + "-action"
@@ -181,13 +190,11 @@
181190
document.getElementById(chkbox).innerHTML = ""
182191
if (result["response"][request_id]["success"]) {
183192
msg = result["response"][request_id]["msg"]
184-
if (document.getElementById(div_id))
185-
document.getElementById(div_id).innerHTML = "<p style='color: #68e068'>" + msg + "</p>"
193+
setColoredMessage(document.getElementById(div_id), msg, "#68e068")
186194
}
187195
else {
188196
error = result["response"][request_id]["error"];
189-
if (document.getElementById(div_id))
190-
document.getElementById(div_id).innerHTML = "<p style='color: red'>" + error + "</p>"
197+
setColoredMessage(document.getElementById(div_id), error, "red")
191198
}
192199
}
193200
if (selector.endsWith("-club") || selector == "clubGroupAccess") {
@@ -278,7 +285,7 @@
278285
},
279286
error: function (result) {
280287
error = result["responseJSON"]["error"]
281-
document.getElementById(div_id).innerHTML = "<p style='color: red'>" + error + "</p>"
288+
setColoredMessage(document.getElementById(div_id), error, "red")
282289
modal.style.display = "none";
283290
document.getElementById("decline-processing").innerHTML = ""
284291
}
@@ -297,7 +304,7 @@
297304
request_id = $(this).val()
298305
modal.style.display = "block";
299306
$('#declineUrlParams').val(selector)
300-
document.getElementById("declineHeading").innerHTML = "Decline " + request_id
307+
document.getElementById("declineHeading").textContent = "Decline " + request_id
301308
});
302309

303310
$('#declineReason').on('change', function () {

0 commit comments

Comments
 (0)