[JENKINS-69659] Un-inline multiple occurrences of JavaScript in Jelly templates #130
Conversation
| .append(jsStringEscape("/><label class='attach-previous'>")).append(escapedName).append(" (").append(escapedDescription).append(")</label>\"") | ||
| .toString(); | ||
| // '${h.jsStringEscape('<input type="checkbox" name="values" json="'+h.htmlAttributeEscape(l.name)+'" ')}'+has("${h.jsStringEscape(l.name)}")+'${h.jsStringEscape('/><label class="attach-previous">'+l.name+' ('+l.description+')</label>')}' | ||
| return "<input type='checkbox' name='values' json='" + |
There was a problem hiding this comment.
Can revert this if there are objections, this was a refactoring suggested by IDE.
There was a problem hiding this comment.
FTR That code was a correction of a previous vulnerability, not sure the current version does not reintroduce issues.
With new YAHOO.widget.HTMLNode I am pretty sure it interprets the values as HTML. Have you tried to reproduce the previous vulnerability or to inject some payloads to understand the behavior? :)
There was a problem hiding this comment.
I think lines 89 and 90 are related to the vulnerability fix, unless you mean a different one that I'm not aware of.
I've checked with simple payloads I could think of, and content was escaped properly. I'll give it another shot too to make sure.
There was a problem hiding this comment.
I've re-checked, it seems fine to me. Tried couple different payloads, couldn't break it. User provided content is escaped. I'd appreciate someone else trying to break it though.
| @@ -1083,6 +1084,10 @@ public List<AxisDescriptor> getAxisDescriptors() { | |||
| return r; | |||
| } | |||
|
|
|||
| public FormValidation doCheckDisplayName(@QueryParameter String value, @QueryParameter String name) { | |||
| return Jenkins.get().doCheckDisplayName(value, name); | |||
There was a problem hiding this comment.
There are some limitations in checkDependsOn, and one of the ways around it was to proxy call this. I'm open to suggestions if anyone has an idea how to do it cleaner.
src/main/resources/hudson/matrix/LabelAxis/label-axis-resources.js
Outdated
Show resolved
Hide resolved
| .append(jsStringEscape("/><label class='attach-previous'>")).append(escapedName).append(" (").append(escapedDescription).append(")</label>\"") | ||
| .toString(); | ||
| // '${h.jsStringEscape('<input type="checkbox" name="values" json="'+h.htmlAttributeEscape(l.name)+'" ')}'+has("${h.jsStringEscape(l.name)}")+'${h.jsStringEscape('/><label class="attach-previous">'+l.name+' ('+l.description+')</label>')}' | ||
| return "<input type='checkbox' name='values' json='" + |
There was a problem hiding this comment.
FTR That code was a correction of a previous vulnerability, not sure the current version does not reintroduce issues.
With new YAHOO.widget.HTMLNode I am pretty sure it interprets the values as HTML. Have you tried to reproduce the previous vulnerability or to inject some payloads to understand the behavior? :)
|
Merge conflicts @yaroslavafenkin; and @Wadeck did you plan to review again? |
|
No review required from me 👍 |
| @@ -1083,6 +1085,11 @@ public List<AxisDescriptor> getAxisDescriptors() { | |||
| return r; | |||
| } | |||
|
|
|||
| @Restricted(NoExternalUse.class) | |||
There was a problem hiding this comment.
With jenkinsci/jenkins#9150 the method becomes obsolete
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -1125,6 +1127,11 @@ | |||
| return r; | |||
| } | |||
|
|
|||
| @Restricted(NoExternalUse.class) | |||
| public FormValidation doCheckDisplayNameOrNull(@AncestorInPath MatrixProject job, @QueryParameter String value) { | |||
Check warning
Code scanning / Jenkins Security Scan
Stapler: Missing POST/RequirePOST annotation Warning
| @@ -1125,6 +1127,11 @@ | |||
| return r; | |||
| } | |||
|
|
|||
| @Restricted(NoExternalUse.class) | |||
| public FormValidation doCheckDisplayNameOrNull(@AncestorInPath MatrixProject job, @QueryParameter String value) { | |||
Check warning
Code scanning / Jenkins Security Scan
Stapler: Missing permission check Warning
basil
changed the title
|
Passing ATH: jenkinsci/acceptance-test-harness#1747 |
|
Passing PCT: jenkinsci/bom#3652 |
JENKINS-69659