CVE-2019-1003042

@@ -24,22 +24,33 @@
 						<td class="pane-header">Labels</td>
 						<td class="pane-header">Action</td>
 					</tr>
+<script>
+function find_resource_name(element) {
+	var row = element.up('tr');
+	var resourceName = row.getAttribute('data-resource-name');
+	return resourceName;
+}
+</script>
 <j:forEach var="resource" items="${it.resources}" indexVar="i">
 <script>
-function unlock_resource_${i}() {
-	window.location.assign("unlock?resource=${resource.name}");
+function unlock_resource_${i}(button) {
+	var resourceName = find_resource_name(button);
+	window.location.assign("unlock?resource=" + resourceName);
 }
-function reserve_resource_${i}() {
-	window.location.assign("reserve?resource=${resource.name}");
+function reserve_resource_${i}(button) {
+	var resourceName = find_resource_name(button);
+	window.location.assign("reserve?resource=" + resourceName);
 }
-function unreserve_resource_${i}() {
-	window.location.assign("unreserve?resource=${resource.name}");
+function unreserve_resource_${i}(button) {
+	var resourceName = find_resource_name(button);
+	window.location.assign("unreserve?resource=" + resourceName);
 }
-function reset_resource_${i}() {
-	window.location.assign("reset?resource=${resource.name}");
+function reset_resource_${i}(button) {
+	var resourceName = find_resource_name(button);
+	window.location.assign("reset?resource=" + resourceName);
 }
 </script>
-					<tr>
+					<tr data-resource-name="${resource.name}">
 						<td class="pane">
 							<strong>${resource.name}</strong><br/>
 							<em>${resource.description}</em>
@@ -54,7 +65,7 @@ function reset_resource_${i}() {
 						<td class="pane">${resource.labels}</td>
 						<td class="pane">
 <j:if test="${h.hasPermission(it.UNLOCK)}">
-							<button onClick="unlock_resource_${i}();">Unlock</button>
+							<button onClick="unlock_resource_${i}(this);">Unlock</button>
 </j:if>
 						</td>
 </j:if>
@@ -67,7 +78,7 @@ function reset_resource_${i}() {
 						<td class="pane">
 		<j:if test="${h.hasPermission(it.RESERVE)}">
 			<j:if test="${it.UserName == resource.reservedBy or h.hasPermission(app.ADMINISTER)}">
-							<button onClick="unreserve_resource_${i}();">UnReserve</button>
+							<button onClick="unreserve_resource_${i}(this);">UnReserve</button>
 			</j:if>
 		</j:if>
 						</td>
@@ -80,7 +91,7 @@ function reset_resource_${i}() {
 						<td class="pane">${resource.labels}</td>
 						<td class="pane">
 			<j:if test="${h.hasPermission(it.UNLOCK)}">
-							<button onClick="reset_resource_${i}();">ResetResource</button>
+							<button onClick="reset_resource_${i}(this);">ResetResource</button>
 			</j:if>
 						</td>
 		</j:if>
@@ -91,7 +102,7 @@ function reset_resource_${i}() {
 						<td class="pane">${resource.labels}</td>
 						<td class="pane">
 			<j:if test="${h.hasPermission(it.RESERVE) and it.UserName != null}">
-								<button onClick="reserve_resource_${i}();">Reserve</button>
+								<button onClick="reserve_resource_${i}(this);">Reserve</button>
 			</j:if>
 						</td>
 		</j:if>

@@ -0,0 +1,104 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2019, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.jenkins.plugins.lockableresources;
+
+import com.gargoylesoftware.htmlunit.AlertHandler;
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import com.gargoylesoftware.htmlunit.Page;
+import com.gargoylesoftware.htmlunit.html.HtmlButton;
+import com.gargoylesoftware.htmlunit.html.HtmlElementUtil;
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import hudson.security.FullControlOnceLoggedInAuthorizationStrategy;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
+
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.junit.Assert.assertThat;
+
+public class LockableResourceRootActionSEC1361Test {
+    
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+    
+    @Test
+    public void regularCase() throws Exception {
+        checkXssWithResourceName("resource1");
+    }
+    
+    @Test
+    @Issue("SECURITY-1361")
+    public void noXssOnClick() throws Exception {
+        checkXssWithResourceName("\"); alert(123);//");
+    }
+    
+    private void checkXssWithResourceName(String resourceName) throws Exception {
+        LockableResourcesManager.get().createResource(resourceName);
+        
+        j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+        j.jenkins.setAuthorizationStrategy(new FullControlOnceLoggedInAuthorizationStrategy());
+        
+        JenkinsRule.WebClient wc = j.createWebClient();
+        wc.login("user");
+        
+        final AtomicReference<String> lastAlertReceived = new AtomicReference<>();
+        wc.setAlertHandler(new AlertHandler() {
+            @Override
+            public void handleAlert(Page page, String s) {
+                lastAlertReceived.set(s);
+            }
+        });
+        
+        HtmlPage htmlPage = wc.goTo("lockable-resources");
+        assertThat(lastAlertReceived.get(), nullValue());
+        
+        // currently only one button but perhaps in future version of the core/plugin, 
+        // other buttons will be added to the layout
+        List<HtmlButton> allButtons = htmlPage.getDocumentElement().getHtmlElementsByTagName("button");
+        assertThat(allButtons.size(), greaterThanOrEqualTo(1));
+        
+        HtmlButton reserveButton = null;
+        for (HtmlButton b : allButtons) {
+            String onClick = b.getAttribute("onClick");
+            if (onClick != null && onClick.contains("reserve_resource")) {
+                reserveButton = b;
+            }
+        }
+        assertThat(reserveButton, not(nullValue()));
+        
+        try {
+            HtmlElementUtil.click(reserveButton);
+        } catch (FailingHttpStatusCodeException e) {
+            // only happen if we have a XSS, but it's managed using the AlertHandler to ensure it's a XSS
+            // and not just an invalid page
+        }
+        assertThat(lastAlertReceived.get(), nullValue());
+    }
+}