Skip to content

Commit 069382b

Browse files
irtazaakramirtazaakram
committed
fix: codeql issues
1 parent dd866be commit 069382b

3 files changed

Lines changed: 60 additions & 73 deletions

File tree

‎xmodule/js/src/capa/display.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -779,8 +779,7 @@
779779
var answers;
780780
answers = response.answers;
781781
$.each(answers, function (key, value) {
782-
var safeKey = key.replace(":", "\\:"); // fix for courses which use url_names with colons, e.g. problem:question1
783-
safeKey = safeKey.replace(/\./g, "\\."); // fix for courses which use url_names with periods. e.g. question1.1
782+
var safeKey = key.replace(/\\/g, "\\\\").replace(/:/g, "\\:").replace(/\./g, "\\."); // fix for courses which use url_names with colons & periods, e.g. problem:question1, question1.1
784783
var answer;
785784
if (!$.isArray(value)) {
786785
answer = that.$("#answer_" + safeKey + ", #solution_" + safeKey);
@@ -1141,8 +1140,7 @@
11411140
var answer, choice, inputId, i, len, results, $element, $inputLabel, $inputStatus;
11421141
$element = $(element);
11431142
inputId = $element.attr("id").replace(/inputtype_/, "");
1144-
inputId = inputId.replace(":", "\\:"); // fix for courses which use url_names with colons, e.g. problem:question1
1145-
var safeId = inputId.replace(/\./g, "\\."); // fix for courses which use url_names with periods. e.g. question1.1
1143+
var safeId = inputId.replace(/\\/g, "\\\\").replace(/:/g, "\\:").replace(/\./g, "\\."); // fix for courses which use url_names with colons & periods, e.g. problem:question1, question1.1
11461144
answer = answers[inputId];
11471145
results = [];
11481146
for (i = 0, len = answer.length; i < len; i++) {

‎xmodule/js/src/javascript_loader.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@
6868
// into memory; (2) delete the DOM script element; (3) reappend it.
6969
// This would prevent memory bloat and save a network request.
7070
src = $(placeholder).attr("data-src");
71-
if (!(src in loaded)) {
71+
// FIX: Validate protocol to prevent XSS. Invalid URLs fall through to 'else' and are skipped safely.
72+
if (!(src in loaded) && /^(https?:\/\/|\/|\.\/|\.\.\/)/i.test(src)) {
7273
loaded[src] = true;
7374
s = document.createElement("script");
7475
s.setAttribute("src", src);

‎xmodule/js/src/problem/edit.js‎

Lines changed: 56 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,8 @@
364364
if (result.length === 1 || !result[1]) {
365365
return label;
366366
}
367-
// xss-lint: disable=javascript-concat-html
368-
return label + "<description>" + result[1] + "</description>\n";
367+
368+
return label + "<description>" + result[1] + "</description>\n"; // xss-lint: disable=javascript-concat-html
369369
});
370370

371371
// Pull out demand hints, || a hint ||
@@ -377,8 +377,7 @@
377377
for (i = 0; i < options.length; i += 1) {
378378
inner = /\s*\|\|(.*?)\|\|/.exec(options[i]);
379379
if (inner) {
380-
// xss-lint: disable=javascript-concat-html
381-
demandhints += " <hint>" + inner[1].trim() + "</hint>\n";
380+
demandhints += " <hint>" + inner[1].trim() + "</hint>\n"; // xss-lint: disable=javascript-concat-html
382381
}
383382
}
384383
return "";
@@ -461,8 +460,8 @@
461460
optiontag += correct[1];
462461
}
463462
optiontag += '">';
464-
// xss-lint: disable=javascript-concat-html
465-
return "\n<optionresponse>\n" + optiontag + "</optioninput>\n</optionresponse>\n\n";
463+
464+
return "\n<optionresponse>\n" + optiontag + "</optioninput>\n</optionresponse>\n\n"; // xss-lint: disable=javascript-concat-html
466465
}
467466

468467
// new style [[ many-lines ]]
@@ -482,15 +481,15 @@
482481
if (label) {
483482
label = ' label="' + label + '"';
484483
}
485-
// xss-lint: disable=javascript-concat-html
486-
hintstr = " <optionhint" + label + ">" + textHint.hint + "</optionhint>";
484+
485+
hintstr = " <optionhint" + label + ">" + textHint.hint + "</optionhint>"; // xss-lint: disable=javascript-concat-html
487486
}
488-
// xss-lint: disable=javascript-concat-html
489-
optionlines += " <option" + correctstr + ">" + textHint.nothint + hintstr + "</option>\n";
487+
488+
optionlines += " <option" + correctstr + ">" + textHint.nothint + hintstr + "</option>\n"; // xss-lint: disable=javascript-concat-html
490489
}
491490
}
492-
// xss-lint: disable=javascript-concat-html
493-
return "\n<optionresponse>\n <optioninput>\n" + optionlines + " </optioninput>\n</optionresponse>\n\n";
491+
492+
return "\n<optionresponse>\n <optioninput>\n" + optionlines + " </optioninput>\n</optionresponse>\n\n"; // xss-lint: disable=javascript-concat-html
494493
});
495494

496495
// multiple choice questions
@@ -525,11 +524,11 @@
525524
hint = extractHint(value);
526525
if (hint.hint) {
527526
value = hint.nothint;
528-
// xss-lint: disable=javascript-concat-html
529-
value = value + " <choicehint" + hint.labelassign + ">" + hint.hint + "</choicehint>";
527+
528+
value = value + " <choicehint" + hint.labelassign + ">" + hint.hint + "</choicehint>"; // xss-lint: disable=javascript-concat-html
530529
}
531-
// xss-lint: disable=javascript-concat-html
532-
choices += ' <choice correct="' + correct + '"' + fixed + ">" + value + "</choice>\n";
530+
531+
choices += ' <choice correct="' + correct + '"' + fixed + ">" + value + "</choice>\n"; // xss-lint: disable=javascript-concat-html
533532
}
534533
}
535534
result = "<multiplechoiceresponse>\n";
@@ -572,8 +571,8 @@
572571
// lone case of hint text processing outside of extractHint, since syntax here is unique
573572
hintbody = abhint[2];
574573
hintbody = hintbody.replace("&lf;", "\n").trim();
575-
// xss-lint: disable=javascript-concat-html
576-
endHints += ' <compoundhint value="' + abhint[1].trim() + '">' + hintbody + "</compoundhint>\n";
574+
575+
endHints += ' <compoundhint value="' + abhint[1].trim() + '">' + hintbody + "</compoundhint>\n"; // xss-lint: disable=javascript-concat-html
577576
// eslint-disable-next-line no-continue
578577
continue; // bail
579578
}
@@ -595,13 +594,11 @@
595594
// checkbox choicehints get their own line, since there can be two of them
596595
// <choicehint selected="true">You’re right that apple is a fruit.</choicehint>
597596
if (select) {
598-
// xss-lint: disable=javascript-concat-html
599-
hints += '\n <choicehint selected="true">' + select[2].trim() + "</choicehint>";
597+
hints += '\n <choicehint selected="true">' + select[2].trim() + "</choicehint>"; // xss-lint: disable=javascript-concat-html
600598
}
601599
select = /{\s*(u|unselected):((.|\n)*?)}/i.exec(inner);
602600
if (select) {
603-
// xss-lint: disable=javascript-concat-html
604-
hints += '\n <choicehint selected="false">' + select[2].trim() + "</choicehint>";
601+
hints += '\n <choicehint selected="false">' + select[2].trim() + "</choicehint>"; // xss-lint: disable=javascript-concat-html
605602
}
606603

607604
// Blank out the original text only if the specific "selected" syntax is found
@@ -610,8 +607,8 @@
610607
value = hint.nothint;
611608
}
612609
}
613-
// xss-lint: disable=javascript-concat-html
614-
groupString += ' <choice correct="' + correct + '">' + value + hints + "</choice>\n";
610+
611+
groupString += ' <choice correct="' + correct + '">' + value + hints + "</choice>\n"; // xss-lint: disable=javascript-concat-html
615612
}
616613
}
617614

@@ -676,24 +673,23 @@
676673
hintLine = "";
677674
if (textHint.hint) {
678675
firstAnswer = textHint.nothint;
679-
// xss-lint: disable=javascript-concat-html
680-
hintLine = " <correcthint" + textHint.labelassign + ">" + textHint.hint + "</correcthint>\n";
676+
677+
hintLine = " <correcthint" + textHint.labelassign + ">" + textHint.hint + "</correcthint>\n"; // xss-lint: disable=javascript-concat-html
681678
}
682679

683680
// Range case
684681
if (isRangeToleranceCase(firstAnswer)) {
685682
// [5, 7) or (5, 7), or (1.2345 * (2+3), 7*4 ] - range tolerance case
686683
// = (5*2)*3 should not be used as range tolerance
687-
// xss-lint: disable=javascript-concat-html
688-
numericalResponseString = '<numericalresponse answer="' + firstAnswer + '">\n';
684+
685+
numericalResponseString = '<numericalresponse answer="' + firstAnswer + '">\n'; // xss-lint: disable=javascript-concat-html
689686
} else {
690687
answerData = getAnswerData(firstAnswer);
691-
// xss-lint: disable=javascript-concat-html
692-
numericalResponseString = '<numericalresponse answer="' + answerData.answer + '">\n';
688+
689+
numericalResponseString = '<numericalresponse answer="' + answerData.answer + '">\n'; // xss-lint: disable=javascript-concat-html
693690
if (answerData.default) {
694-
// xss-lint: disable=javascript-concat-html
695691
numericalResponseString +=
696-
' <responseparam type="tolerance" default="' + answerData.default + '" />\n';
692+
' <responseparam type="tolerance" default="' + answerData.default + '" />\n'; // xss-lint: disable=javascript-concat-html
697693
}
698694
}
699695

@@ -715,17 +711,15 @@
715711
}
716712

717713
if (additionalTextHint.hint) {
718-
// xss-lint: disable=javascript-concat-html
719714
additionalHintLine =
720-
"<correcthint" +
721-
additionalTextHint.labelassign +
722-
">" +
723-
additionalTextHint.hint +
724-
"</correcthint>";
715+
"<correcthint" + // xss-lint: disable=javascript-concat-html
716+
additionalTextHint.labelassign + // xss-lint: disable=javascript-concat-html
717+
">" + // xss-lint: disable=javascript-concat-html
718+
additionalTextHint.hint + // xss-lint: disable=javascript-concat-html
719+
"</correcthint>"; // xss-lint: disable=javascript-concat-html
725720
}
726721

727-
// xss-lint: disable=javascript-concat-html
728-
additionalAnswerString += ' <additional_answer answer="' + orMatch[1] + '">';
722+
additionalAnswerString += ' <additional_answer answer="' + orMatch[1] + '">'; // xss-lint: disable=javascript-concat-html
729723
additionalAnswerString += additionalHintLine;
730724
additionalAnswerString += "</additional_answer>\n";
731725
}
@@ -755,10 +749,9 @@
755749
typ = ' type="ci regexp"';
756750
firstAnswer = firstAnswer.slice(1).trim();
757751
}
758-
// xss-lint: disable=javascript-concat-html
759-
string = '<stringresponse answer="' + firstAnswer + '"' + typ + " >\n";
752+
753+
string = '<stringresponse answer="' + firstAnswer + '"' + typ + " >\n"; // xss-lint: disable=javascript-concat-html
760754
if (textHint.hint) {
761-
// xss-lint: disable=javascript-concat-html
762755
string += " <correcthint" + textHint.labelassign + ">" + textHint.hint + "</correcthint>\n"; // xss-lint: disable=javascript-concat-html
763756
}
764757

@@ -767,27 +760,25 @@
767760
textHint = extractHint(values[i]);
768761
notMatch = /^not\=\s*(.*)/.exec(textHint.nothint);
769762
if (notMatch) {
770-
// xss-lint: disable=javascript-concat-html
771763
string +=
772-
' <stringequalhint answer="' +
773-
notMatch[1] +
774-
'"' +
775-
textHint.labelassign +
776-
">" +
777-
textHint.hint +
778-
"</stringequalhint>\n";
764+
' <stringequalhint answer="' + // xss-lint: disable=javascript-concat-html
765+
notMatch[1] + // xss-lint: disable=javascript-concat-html
766+
'"' + // xss-lint: disable=javascript-concat-html
767+
textHint.labelassign + // xss-lint: disable=javascript-concat-html
768+
">" + // xss-lint: disable=javascript-concat-html
769+
textHint.hint + // xss-lint: disable=javascript-concat-html
770+
"</stringequalhint>\n"; // xss-lint: disable=javascript-concat-html
779771

780772
// eslint-disable-next-line no-continue
781773
continue;
782774
}
783775
orMatch = /^or\=\s*(.*)/.exec(textHint.nothint);
784776
if (orMatch) {
785777
// additional_answer with answer= attribute
786-
// xss-lint: disable=javascript-concat-html
787-
string += ' <additional_answer answer="' + orMatch[1] + '">';
778+
779+
string += ' <additional_answer answer="' + orMatch[1] + '">'; // xss-lint: disable=javascript-concat-html
788780
if (textHint.hint) {
789-
// xss-lint: disable=javascript-concat-html
790-
string += "<correcthint" + textHint.labelassign + ">" + textHint.hint + "</correcthint>";
781+
string += "<correcthint" + textHint.labelassign + ">" + textHint.hint + "</correcthint>"; // xss-lint: disable=javascript-concat-html
791782
}
792783
string += "</additional_answer>\n";
793784
}
@@ -803,24 +794,22 @@
803794

804795
// replace explanations
805796
xml = xml.replace(/\[explanation\]\n?([^\]]*)\[\/?explanation\]/gim, function (match, p1) {
806-
// xss-lint: disable=javascript-concat-html
807797
return (
808-
'<solution>\n<div class="detailed-solution">\n' +
798+
'<solution>\n<div class="detailed-solution">\n' + // xss-lint: disable=javascript-concat-html
809799
gettext("Explanation") +
810-
"\n\n" +
811-
p1 +
800+
"\n\n" + // xss-lint: disable=javascript-concat-html
801+
p1 + // xss-lint: disable=javascript-concat-html
812802
"\n</div>\n</solution>"
813803
);
814804
});
815805

816806
// replace code blocks
817807
xml = xml.replace(/\[code\]\n?([^\]]*)\[\/?code\]/gim, function (match, p1) {
818-
// xss-lint: disable=javascript-concat-html
819-
return "<pre><code>" + p1 + "</code></pre>";
808+
return "<pre><code>" + p1 + "</code></pre>"; // xss-lint: disable=javascript-concat-html
820809
});
821810

822811
// split scripts and preformatted sections, and wrap paragraphs
823-
splits = xml.split(/(\<\/?(?:script|pre|label|description).*?\>)/g);
812+
splits = xml.split(/(<\/?(?:script|pre|label|description)[^>]*>)/gi);
824813

825814
// Wrap a string by <p> tag when line is not already wrapped by another tag
826815
// true when line is not already wrapped by another tag false otherwise
@@ -854,8 +843,8 @@
854843
responseTypesSelector = responseTypes.join(", ");
855844

856845
// make temporary xml
857-
// xss-lint: disable=javascript-concat-html
858-
$xml = $($.parseXML("<prob>" + xml + "</prob>"));
846+
847+
$xml = $($.parseXML("<prob>" + xml + "</prob>")); // xss-lint: disable=javascript-concat-html
859848
responseType = $xml.find(responseTypesSelector);
860849

861850
// convert if there is only one responsetype
@@ -903,12 +892,11 @@
903892
});
904893
finalDemandHints = "";
905894
if (demandHintTags.length) {
906-
// xss-lint: disable=javascript-concat-html
907-
finalDemandHints = "\n<demandhint>\n" + demandHintTags.join("") + "</demandhint>";
895+
finalDemandHints = "\n<demandhint>\n" + demandHintTags.join("") + "</demandhint>"; // xss-lint: disable=javascript-concat-html
908896
}
909897
// make all responsetypes descendants of a single problem element
910-
// xss-lint: disable=javascript-concat-html
911-
finalXml = "<problem>\n" + responseTypesXML.join("\n\n") + finalDemandHints + "\n</problem>";
898+
899+
finalXml = "<problem>\n" + responseTypesXML.join("\n\n") + finalDemandHints + "\n</problem>"; // xss-lint: disable=javascript-concat-html
912900
return finalXml;
913901
};
914902

0 commit comments

Comments
 (0)