-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[JENKINS-69659] Un-inline multiple occurrences of JavaScript in Jelly templates #130
Changes from all commits
a0b9176
e7ee4ff
90cf96f
4fc157a
c108a06
bd0cea2
5454028
cc545f6
4f1c596
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,7 +108,9 @@ | |
|
||
import org.kohsuke.accmod.Restricted; | ||
import org.kohsuke.accmod.restrictions.NoExternalUse; | ||
import org.kohsuke.stapler.AncestorInPath; | ||
import org.kohsuke.stapler.HttpResponse; | ||
import org.kohsuke.stapler.QueryParameter; | ||
import org.kohsuke.stapler.StaplerRequest; | ||
import org.kohsuke.stapler.StaplerResponse; | ||
import org.kohsuke.stapler.TokenList; | ||
|
@@ -1125,6 +1127,11 @@ | |
return r; | ||
} | ||
|
||
@Restricted(NoExternalUse.class) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With jenkinsci/jenkins#9150 the method becomes obsolete |
||
public FormValidation doCheckDisplayNameOrNull(@AncestorInPath MatrixProject job, @QueryParameter String value) { | ||
Check warning Code scanning / Jenkins Security Scan Stapler: Missing POST/RequirePOST annotation Warning
Potential CSRF vulnerability: If DescriptorImpl#doCheckDisplayNameOrNull connects to user-specified URLs, modifies state, or is expensive to run, it should be annotated with @POST or @RequirePOST
Check warning Code scanning / Jenkins Security Scan Stapler: Missing permission check Warning
Potential missing permission check in DescriptorImpl#doCheckDisplayNameOrNull
|
||
return Jenkins.get().doCheckDisplayName(value, job.getName()); | ||
} | ||
|
||
/** | ||
* @deprecated as of 1.456 | ||
* This was only exposed for Jelly. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
Behaviour.specify("DIV.labelAxis-tree", 'LabelAxis', 0, function(e) { | ||
var tree = new YAHOO.widget.TreeView(e); | ||
|
||
var i18nContainer = document.querySelector(".label-axis-i18n"); | ||
var labels = new YAHOO.widget.TextNode(i18nContainer.getAttribute("data-i18n-labels"), tree.getRoot(), false); | ||
var machines = new YAHOO.widget.TextNode(i18nContainer.getAttribute("data-i18n-individual-nodes"), tree.getRoot(), false); | ||
|
||
var values = (e.getAttribute("values") || "").split("/"); | ||
function has(v) { | ||
return values.includes(v) ? 'checked="checked" ' : ""; | ||
} | ||
|
||
var labelAxisDataContainer = document.querySelector(".label-axis-data-container"); | ||
labelAxisDataContainer.childNodes.forEach(node => { | ||
var labelCheckbox = node.getAttribute("data-label-checkbox"); | ||
|
||
var CHECKED_ATTR_INSERT_IDX = "<input ".length; | ||
var output = [labelCheckbox.slice(0, CHECKED_ATTR_INSERT_IDX), | ||
has(node.getAttribute("data-label-atom")), labelCheckbox.slice(CHECKED_ATTR_INSERT_IDX)].join(''); | ||
var label = node.getAttribute("data-label"); | ||
new YAHOO.widget.HTMLNode(output, label === "machines" ? machines : labels, false); | ||
}); | ||
|
||
tree.draw(); | ||
/* | ||
force the rendering of HTML, so that input fields are there | ||
even when the form is submitted without this tree expanded. | ||
*/ | ||
tree.expandAll(); | ||
tree.collapseAll(); | ||
|
||
/* | ||
cancel the event. | ||
from http://yuilibrary.com/forum/viewtopic.php?f=89&t=8209&p=26239&hilit=HTMLNode#p26239 | ||
"To prevent toggling and allow the link to work, add a listener to the clickEvent on that tree and simply return false" | ||
*/ | ||
tree.subscribe("clickEvent", function(node) { | ||
return false; | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
refreshPart('matrix',"./ajaxMatrix"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can revert this if there are objections, this was a refactoring suggested by IDE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.