Skip to content

Update jdom2 to 2.0.6.1#699

Merged
tuxji merged 1 commit into
apache:mainfrom
scala-steward:update/jdom2-2.0.6.1
Dec 15, 2021
Merged

Update jdom2 to 2.0.6.1#699
tuxji merged 1 commit into
apache:mainfrom
scala-steward:update/jdom2-2.0.6.1

Conversation

@scala-steward

Copy link
Copy Markdown
Contributor

Updates org.jdom:jdom2 from 2.0.6 to 2.0.6.1.

I'll automatically update this PR to resolve conflicts as long as you don't change it yourself.

If you'd like to skip this version, you can just close this PR. If you have any feedback, just mention me in the comments below.

Configure Scala Steward for your repository with a .scala-steward.conf file.

Have a fantastic day writing Scala!

Files still referring to the old version number

The following files still refer to the old version number (2.0.6).
You might want to review and update them manually.

project/Dependencies.scala
Ignore future updates

Add this to your .scala-steward.conf file to ignore future updates of this dependency:

updates.ignore = [ { groupId = "org.jdom", artifactId = "jdom2" } ]

labels: library-update, commit-count:1, old-version-remains

@tuxji tuxji left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1

I have visited both http://www.jdom.org/ and https://github.com/hunterhacker/jdom/ and neither say anything about a JDOM 2.6.0.1 release. The GitHub repository has no new commits since October.

@tuxji

tuxji commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

Found more information:

Got security warning on JDom2 2.0.6: CVE-2021-33813 Issue #189
fix setFeature bug and add test case PR #188

Looks like the maintainer just got approval from Sonatype to upload the 2.0.6.1 release, but did not create a tagged release on GitHub (yet?). Still would prefer more traceability of what exactly went into the 2.0.6.1 release back to source tag.

@tuxji

tuxji commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

Found more traceability info. Someone forked the repo, made a branch "2.0.6.1" from the 2.0.6 tag, cherry picked some fixes, and built a jar for Java 1.5 and higher (building a jar for that old Java version had been what took a long time). Then someone else tested the jar, and the maintainer uploaded the jar to Maven Central.

hunterhacker/jdom#189 (comment)
https://github.com/eneklo/jdom/commits/jdom-2.0.6.1

@stevedlawrence

Copy link
Copy Markdown
Member

Hmm, I'm not sure how comfortable I am depending on jar from a random fork that wasn't built by a maintainer. It's probably fine, and a build.log and additional details are included in that issue, so it shouldn't be too hard to verify that there is nothing malicious added to this jar.

@tuxji

tuxji commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

Do we even need the fix? The fix applied only to callers bypassing JDOM's normal API method and calling SAX's setFeature method directly to turn on expansion. However, we do the opposite in Daffodil - we turn expansion off, not on.

@stevedlawrence

Copy link
Copy Markdown
Member

Yeah, I think this technically doesn't affect us.

Though, it would be nice to include this just so that if anyone checks our dependencies a flag won't get raised about us depending on a version of JDOM with open CVE's, even if we aren't affected.

@mbeckerle

Copy link
Copy Markdown
Contributor

So given your observations @tuxji, what triggers scala-steward to provide this as an update.

Feels like there is a substantial secuirty risk if people don't give scala-steward PRs significant scrutiny.

Why would some contributor's fork cause scala-steward to notice?

@tuxji

tuxji commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

In hunterhacker/jdom#189 (comment), the guy who built the jar said:

I built using Apache Ant 1.9.16 and Oracle JDK 1.7.0_75 that is the same version used to build v2.0.6 which is on Maven Central; then I compared the resulting JAR with the one of v2.0.6: there is only one class file changed: SAXBuilder.class; I also compared SAXBuilder.java source with master and there are no code differences (only some fixed typos on master).

However, I downloaded both jdom2-2.0.6.jar and jdom2-2.0.6.1.jar from https://repo1.maven.org/maven2/org/jdom/jdom2/2.0.6/jdom2-2.0.6.jar and https://repo1.maven.org/maven2/org/jdom/jdom2/2.0.6.1/jdom2-2.0.6.1.jar respectively, unpacked them in separate empty directories with "jar xf", and compared them with "diff -rU2". Every class is different and there are 12 new classes in 2.0.6.1. Furthermore, 2.0.6.1 wasn't built with Oracle JDK 1.7.0_75, it was built with Oracle JDK 1.8.0_151. I would have liked the new jar to have the same classes as the original jar (byte-for-byte) except for the class being fixed (SAXBuilder.class).

Let's wait for both the JDOM website and GitHub repository to be updated before bumping the jar. I have left a comment on their issue asking them to announce the 2.0.6.1 release on the JDOM website and upload the jar to their GitHub Release page so that no one else has to spend a lot of additional time checking the jar's validity.

@tuxji

tuxji commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

So given your observations @tuxji, what triggers scala-steward to provide this as an update.

Feels like there is a substantial secuirty risk if people don't give scala-steward PRs significant scrutiny.

Why would some contributor's fork cause scala-steward to notice?

No, scala-steward didn't notice the contributor's fork. What happened is that JDOM had an old maintainer (Rolf) who held the Sonatype account credentials. The new maintainer (Jason) had to get Sonatype account credentials by adding a TXT record to the JDOM website's DNS records, and then uploaded this jar to Sonatype. Scala-steward then noticed the new jar had appeared in Maven Central and sent us a PR.

@tuxji tuxji left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Changes since 2.0.6
Diff since 2.0.6
Fixes CVE-2021-33813

  • Do all automated continuous integration checks pass?
    Yes
  • Is the update a patch, minor, or major update?
    Really a minor update, adds StAX reader/writer, fixes bugs including a CVE
  • Is the license still compatible with ASF License Policy?
    Yes, still BSD (JDOM wording)
  • Have any changes been made to LICENSE/NOTICE files that need to be incorporated?
    No
  • Have any transitive dependencies been added or changed?
    Jaxen optional dependency bumped from 1.1.6 to 1.2.0 (we don't distribute jaxen)

@mbeckerle mbeckerle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@stevedlawrence

Copy link
Copy Markdown
Member

I'm still confused. I thought 2.6.0.1 was supposed to have just a single change for the CVE. But according to the tag on github, there's a bunch of changes:

hunterhacker/jdom@JDOM-2.0.6...JDOM-2.0.6.1

Maybe this is fine, but the history of this release still not clear to me.

@tuxji

tuxji commented Dec 15, 2021

Copy link
Copy Markdown
Contributor

I'm still confused. I thought 2.6.0.1 was supposed to have just a single change for the CVE. But according to the tag on github, there's a bunch of changes:

hunterhacker/jdom@JDOM-2.0.6...JDOM-2.0.6.1

Maybe this is fine, but the history of this release still not clear to me.

Jason Hunter (the current JDOM maintainer) built this jdom-2.0.6.1 jar himself rather than using the jdom-2.0.6.1 jar built by someone who forked the repository. He asked JDOM users to test the jar and also figured out how to upload the jar to Sonatype so that it would be picked up by Maven Central. The jar and its tagged branch are legitimate, even though the tagged branch has more changes than just the single change for the CVE. I agree that the release's contents and history isn't very clear. Let's go ahead and merge this change in, though, so Daffodil won't have an open CVE.

@tuxji tuxji merged commit 5303d86 into apache:main Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants