diff --git a/java/jenkins/authz/permission-redirect-no-return.yaml b/java/jenkins/authz/permission-redirect-no-return.yaml new file mode 100644 index 0000000..b477c5f --- /dev/null +++ b/java/jenkins/authz/permission-redirect-no-return.yaml @@ -0,0 +1,41 @@ +rules: + - id: codevigilant.java.jenkins.authz.permission-redirect-no-return + patterns: + - pattern: | + if (!$OBJ.hasPermission($PERM)) { + ... + $RESP.sendRedirect(...); + ... + } + - pattern-not: | + if (!$OBJ.hasPermission($PERM)) { + ... + return ...; + ... + } + - pattern-not: | + if (!$OBJ.hasPermission($PERM)) { + ... + throw ...; + ... + } + message: | + Detected a negated permission check whose failure branch redirects but + does not terminate the handler with 'return' or 'throw'. The redirect is + cosmetic: execution continues past the branch, so the un-authorized + request proceeds to the protected operations. Every authorization + failure branch must end the request (return / throw) immediately after + the redirect, otherwise the permission check is fail-open. + metadata: + category: security + cwe: "CWE-862: Missing Authorization" + owasp: "A01:2021 - Broken Access Control" + technology: jenkins + confidence: MEDIUM + references: + - https://www.jenkins.io/doc/developer/security/ + source: independent security review + license: MIT + languages: [java] + mode: search + severity: HIGH diff --git a/testcases/java/permission-redirect-no-return-neg.java b/testcases/java/permission-redirect-no-return-neg.java new file mode 100644 index 0000000..b3b14ef --- /dev/null +++ b/testcases/java/permission-redirect-no-return-neg.java @@ -0,0 +1,22 @@ +import hudson.model.AbstractProject; +import hudson.model.Item; +import org.kohsuke.stapler.StaplerRequest; +import org.kohsuke.stapler.StaplerResponse; + +public class PermissionRedirectNeg { + private final AbstractProject project; + + public PermissionRedirectNeg(AbstractProject project) { + this.project = project; + } + + // fixed: terminal return after the redirect + public void doBuild(final StaplerRequest request, final StaplerResponse response) { + if (!project.hasPermission(Item.BUILD)) { + response.sendRedirect(request.getContextPath() + '/' + project.getUrl()); + return; + } + project.scheduleBuild2(0, new Cause.UserIdCause(), new Action[0]); + response.sendRedirect(request.getContextPath() + '/' + project.getUrl()); + } +} diff --git a/testcases/java/permission-redirect-no-return-pos.java b/testcases/java/permission-redirect-no-return-pos.java new file mode 100644 index 0000000..51368d6 --- /dev/null +++ b/testcases/java/permission-redirect-no-return-pos.java @@ -0,0 +1,21 @@ +import hudson.model.AbstractProject; +import hudson.model.Item; +import org.kohsuke.stapler.StaplerRequest; +import org.kohsuke.stapler.StaplerResponse; + +public class PermissionRedirectPos { + private final AbstractProject project; + + public PermissionRedirectPos(AbstractProject project) { + this.project = project; + } + + // vulnerable: redirect without return -> code below runs for un-authorized users + public void doBuild(final StaplerRequest request, final StaplerResponse response) { + if (!project.hasPermission(Item.BUILD)) { + response.sendRedirect(request.getContextPath() + '/' + project.getUrl()); + } + project.scheduleBuild2(0, new Cause.UserIdCause(), new Action[0]); + response.sendRedirect(request.getContextPath() + '/' + project.getUrl()); + } +}