Skip to content

Commit 15c512c

Browse files
authored
[JENKINS-75388] Remove ?path and ?pattern support from DBS (#10650)
Co-authored-by: Daniel Beck <daniel-beck@users.noreply.github.com>
1 parent 36dba29 commit 15c512c

File tree

6 files changed

+88
-21
lines changed

6 files changed

+88
-21
lines changed

core/src/main/java/hudson/model/DirectoryBrowserSupport.java

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -203,15 +203,6 @@ public void serveFile(StaplerRequest req, StaplerResponse rsp, FilePath root, St
203203
}
204204

205205
private void serveFile(StaplerRequest2 req, StaplerResponse2 rsp, VirtualFile root, String icon, boolean serveDirIndex) throws IOException, ServletException, InterruptedException {
206-
// handle form submission
207-
String pattern = req.getParameter("pattern");
208-
if (pattern == null)
209-
pattern = req.getParameter("path"); // compatibility with Hudson<1.129
210-
if (pattern != null && Util.isSafeToRedirectTo(pattern)) { // avoid open redirect
211-
rsp.sendRedirect2(pattern);
212-
return;
213-
}
214-
215206
String path = getPath(req);
216207
if (path.replace('\\', '/').contains("/../")) {
217208
// don't serve anything other than files in the artifacts dir

core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,17 +33,18 @@ THE SOFTWARE.
3333
<div class="dirTree">
3434
<!-- parent path -->
3535
<div class="parentPath">
36-
<form action="${backPath}" method="get">
36+
<form>
3737
<a href="${topPath}">${it.owner.name}</a> /
3838
<j:forEach var="p" items="${parentPath}">
3939
<a href="${p.href}">${p.title}</a>
4040
/
4141
</j:forEach>
42-
<input type="text" name="pattern" value="${pattern}" class="jenkins-input" />
43-
<button class="jenkins-button">
42+
<input type="text" name="pattern" value="${pattern}" class="jenkins-input" id="pattern" data-restofpath="${request2.restOfPath}" data-backpath="${backPath}" />
43+
<button class="jenkins-button" id="pattern-submit">
4444
<l:icon src="symbol-arrow-right" />
4545
</button>
4646
</form>
47+
<st:adjunct includes="hudson.model.DirectoryBrowserSupport.pattern" />
4748
</div>
4849
<j:choose>
4950
<j:when test="${empty(files)}">
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
document.addEventListener("DOMContentLoaded", function () {
2+
document
3+
.getElementById("pattern-submit")
4+
.addEventListener("click", function (ev) {
5+
ev.preventDefault();
6+
7+
let input = document.getElementById("pattern");
8+
let pattern = input.value;
9+
let back = input.dataset.backpath;
10+
11+
let baseurl = back;
12+
if (!baseurl.endsWith("/")) {
13+
baseurl = baseurl + "/";
14+
}
15+
baseurl = baseurl + pattern;
16+
document.location.href = baseurl;
17+
});
18+
});

test/src/test/java/jenkins/security/Security3501Test.java

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,56 @@
44
import static org.hamcrest.Matchers.is;
55

66
import java.util.List;
7+
import jenkins.security.security3501Test.Security3501RootAction;
78
import org.htmlunit.FailingHttpStatusCodeException;
89
import org.junit.Assert;
910
import org.junit.Rule;
1011
import org.junit.Test;
12+
import org.junit.runner.RunWith;
13+
import org.junit.runners.Parameterized;
1114
import org.jvnet.hudson.test.JenkinsRule;
1215
import org.jvnet.hudson.test.RealJenkinsRule;
1316

17+
@RunWith(Parameterized.class)
1418
public class Security3501Test {
19+
// Workaround for https://github.com/jenkinsci/jenkins-test-harness/issues/933
20+
private final String contextPath;
21+
1522
@Rule
16-
public RealJenkinsRule jj = new RealJenkinsRule();
23+
public RealJenkinsRule jj = new RealJenkinsRule().addSyntheticPlugin(new RealJenkinsRule.SyntheticPlugin(Security3501RootAction.class.getPackage()).shortName("Security3501RootAction"));
24+
25+
@Parameterized.Parameters
26+
public static List<String> contexts() {
27+
return List.of("/jenkins", "");
28+
}
29+
30+
public Security3501Test(String contextPath) {
31+
jj.withPrefix(contextPath);
32+
this.contextPath = contextPath;
33+
}
1734

1835
@Test
1936
public void testRedirects() throws Throwable {
20-
jj.then(Security3501Test::_testRedirects);
37+
jj.then(new TestRedirectsStep(contextPath));
2138
}
2239

23-
public static void _testRedirects(JenkinsRule j) throws Exception {
24-
List<String> prohibitedPaths = List.of("%5C%5Cexample.org", "%5C/example.org", "/%5Cexample.org", "//example.org", "https://example.org", "\\example.org");
25-
for (String path : prohibitedPaths) {
26-
try (JenkinsRule.WebClient wc = j.createWebClient().withRedirectEnabled(false)) {
27-
final FailingHttpStatusCodeException fhsce = Assert.assertThrows(FailingHttpStatusCodeException.class, () -> wc.goTo("userContent?path=" + path));
28-
assertThat(fhsce.getStatusCode(), is(302));
29-
assertThat(fhsce.getResponse().getResponseHeaderValue("Location"), is(j.getURL().toExternalForm() + "userContent/"));
40+
private record TestRedirectsStep(String context) implements RealJenkinsRule.Step {
41+
public void run(JenkinsRule j) throws Exception {
42+
List<String> prohibitedPaths = List.of("%5C%5Cexample.org", "%5C/example.org", "/%5Cexample.org", "//example.org", "https://example.org", "\\example.org");
43+
for (String path : prohibitedPaths) {
44+
try (JenkinsRule.WebClient wc = j.createWebClient().withRedirectEnabled(false)) {
45+
final FailingHttpStatusCodeException fhsce = Assert.assertThrows(FailingHttpStatusCodeException.class, () -> wc.goTo("redirects/content?path=" + path));
46+
assertThat(fhsce.getStatusCode(), is(404));
47+
}
48+
}
49+
50+
List<String> allowedPaths = List.of("foo", "foo/bar");
51+
for (String path : allowedPaths) {
52+
try (JenkinsRule.WebClient wc = j.createWebClient().withRedirectEnabled(false)) {
53+
final FailingHttpStatusCodeException fhsce = Assert.assertThrows(FailingHttpStatusCodeException.class, () -> wc.goTo("redirects/content?path=" + path));
54+
assertThat(fhsce.getStatusCode(), is(302));
55+
assertThat(fhsce.getResponse().getResponseHeaderValue("Location"), is(context + "/redirects/" + path));
56+
}
3057
}
3158
}
3259
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package jenkins.security.security3501Test;
2+
3+
import hudson.Util;
4+
import hudson.model.InvisibleAction;
5+
import hudson.model.RootAction;
6+
import java.io.IOException;
7+
import org.jenkinsci.plugins.variant.OptionalExtension;
8+
import org.kohsuke.stapler.StaplerRequest2;
9+
import org.kohsuke.stapler.StaplerResponse2;
10+
11+
@OptionalExtension
12+
public class Security3501RootAction extends InvisibleAction implements RootAction {
13+
@Override
14+
public String getUrlName() {
15+
return "redirects";
16+
}
17+
18+
public void doContent(StaplerRequest2 req, StaplerResponse2 rsp) throws IOException {
19+
final String path = req.getParameter("path");
20+
if (path != null && Util.isSafeToRedirectTo(path)) {
21+
rsp.sendRedirect2(path);
22+
return;
23+
}
24+
rsp.setStatus(404);
25+
}
26+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
@OptionalPackage(requirePlugins = "Security3501RootAction")
2+
package jenkins.security.security3501Test;
3+
4+
import org.jenkinsci.plugins.variant.OptionalPackage;

0 commit comments

Comments
 (0)