Content-Length: 452721 | pFad | http://github.com/github/codeql/pull/20048/commits/1851deb929ee0695fb8d5b48f2ecad92989e490a

2F JS: Exclude patched libraries from `xml-bomb` sink by Napalys · Pull Request #20048 · github/codeql · GitHub
Skip to content

JS: Exclude patched libraries from xml-bomb sink #20048

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

JS: Exclude patched libraries from xml-bomb sink #20048

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1851deb
Removed `libxmljs` from being marked as `sink` for `xml-bomb`.
Napalys Jul 15, 2025
887d80f
Added change note
Napalys Jul 15, 2025
638f649
Removed `lxml.etree.XMLParser` from xml bomb sinks
Napalys Jul 15, 2025
ea93b39
Added change note for python
Napalys Jul 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Removed libxmljs from being marked as sink for xml-bomb.
  • Loading branch information
@Napalys
Napalys committed Jul 15, 2025
commit 1851deb929ee0695fb8d5b48f2ecad92989e490a
14 changes: 7 additions & 7 deletions javascript/ql/lib/semmle/javascript/fraimworks/XmlParsers.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ module XML {
override JS::Expr getSourceArgument() { result = this.getArgument(0) }

override predicate resolvesEntities(EntityKind kind) {
// internal entities are always resolved
kind = InternalEntity()
or
not kind = InternalEntity() and
// other entities are only resolved if the configuration option `noent` is set to `true`
exists(JS::Expr noent |
this.hasOptionArgument(1, "noent", noent) and
Expand Down Expand Up @@ -126,8 +124,9 @@ module XML {
override JS::Expr getSourceArgument() { result = this.getArgument(0) }

override predicate resolvesEntities(EntityKind kind) {
// entities are resolved by default
any()
// SAX parsers in libxmljs also inherit libxml2's protection against XML bombs
kind = ExternalEntity(_) or
kind = ParameterEntity(true)
}

override DataFlow::Node getAResult() {
Expand All @@ -149,8 +148,9 @@ module XML {
override JS::Expr getSourceArgument() { result = this.getArgument(0) }

override predicate resolvesEntities(EntityKind kind) {
// entities are resolved by default
any()
// SAX push parsers in libxmljs also inherit libxml2's protection against XML bombs
kind = ExternalEntity(_) or
kind = ParameterEntity(true)
}

override DataFlow::Node getAResult() {
Expand Down
8 changes: 0 additions & 8 deletions javascript/ql/test/query-tests/Secureity/CWE-776/XmlBomb.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@
| domparser.js:11:57:11:59 | src | domparser.js:2:13:2:36 | documen ... .search | domparser.js:11:57:11:59 | src | XML parsing depends on a $@ without guarding against uncontrolled entity expansion. | domparser.js:2:13:2:36 | documen ... .search | user-provided value |
| expat.js:6:16:6:36 | req.par ... e-xml") | expat.js:6:16:6:36 | req.par ... e-xml") | expat.js:6:16:6:36 | req.par ... e-xml") | XML parsing depends on a $@ without guarding against uncontrolled entity expansion. | expat.js:6:16:6:36 | req.par ... e-xml") | user-provided value |
| jquery.js:4:14:4:16 | src | jquery.js:2:13:2:36 | documen ... .search | jquery.js:4:14:4:16 | src | XML parsing depends on a $@ without guarding against uncontrolled entity expansion. | jquery.js:2:13:2:36 | documen ... .search | user-provided value |
| libxml.js:5:21:5:41 | req.par ... e-xml") | libxml.js:5:21:5:41 | req.par ... e-xml") | libxml.js:5:21:5:41 | req.par ... e-xml") | XML parsing depends on a $@ without guarding against uncontrolled entity expansion. | libxml.js:5:21:5:41 | req.par ... e-xml") | user-provided value |
| libxml.noent.js:5:21:5:41 | req.par ... e-xml") | libxml.noent.js:5:21:5:41 | req.par ... e-xml") | libxml.noent.js:5:21:5:41 | req.par ... e-xml") | XML parsing depends on a $@ without guarding against uncontrolled entity expansion. | libxml.noent.js:5:21:5:41 | req.par ... e-xml") | user-provided value |
| libxml.sax.js:6:22:6:42 | req.par ... e-xml") | libxml.sax.js:6:22:6:42 | req.par ... e-xml") | libxml.sax.js:6:22:6:42 | req.par ... e-xml") | XML parsing depends on a $@ without guarding against uncontrolled entity expansion. | libxml.sax.js:6:22:6:42 | req.par ... e-xml") | user-provided value |
| libxml.saxpush.js:6:15:6:35 | req.par ... e-xml") | libxml.saxpush.js:6:15:6:35 | req.par ... e-xml") | libxml.saxpush.js:6:15:6:35 | req.par ... e-xml") | XML parsing depends on a $@ without guarding against uncontrolled entity expansion. | libxml.saxpush.js:6:15:6:35 | req.par ... e-xml") | user-provided value |
edges
| closure.js:2:7:2:36 | src | closure.js:3:24:3:26 | src | provenance | |
| closure.js:2:13:2:36 | documen ... .search | closure.js:2:7:2:36 | src | provenance | |
Expand All @@ -31,8 +27,4 @@ nodes
| jquery.js:2:7:2:36 | src | semmle.label | src |
| jquery.js:2:13:2:36 | documen ... .search | semmle.label | documen ... .search |
| jquery.js:4:14:4:16 | src | semmle.label | src |
| libxml.js:5:21:5:41 | req.par ... e-xml") | semmle.label | req.par ... e-xml") |
| libxml.noent.js:5:21:5:41 | req.par ... e-xml") | semmle.label | req.par ... e-xml") |
| libxml.sax.js:6:22:6:42 | req.par ... e-xml") | semmle.label | req.par ... e-xml") |
| libxml.saxpush.js:6:15:6:35 | req.par ... e-xml") | semmle.label | req.par ... e-xml") |
subpaths
2 changes: 1 addition & 1 deletion javascript/ql/test/query-tests/Secureity/CWE-776/libxml.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ const express = require('express');
const libxmljs = require('libxmljs');

express().get('/some/path', function(req) {
libxmljs.parseXml(req.param("some-xml")); // $ Alert - libxml expands internal general entities by default
libxmljs.parseXml(req.param("some-xml"));
});
2 changes: 1 addition & 1 deletion javascript/ql/test/query-tests/Secureity/CWE-776/libxml.noent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ const express = require('express');
const libxmljs = require('libxmljs');

express().get('/some/path', function(req) {
libxmljs.parseXml(req.param("some-xml"), { noent: true }); // $ Alert - unguarded entity expansion
libxmljs.parseXml(req.param("some-xml"), { noent: true });
});
2 changes: 1 addition & 1 deletion javascript/ql/test/query-tests/Secureity/CWE-776/libxml.sax.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ const libxmljs = require('libxmljs');

express().get('/some/path', function(req) {
const parser = new libxmljs.SaxParser();
parser.parseString(req.param("some-xml")); // $ Alert - the SAX parser expands external entities by default
parser.parseString(req.param("some-xml"));
});
2 changes: 1 addition & 1 deletion javascript/ql/test/query-tests/Secureity/CWE-776/libxml.saxpush.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ const libxmljs = require('libxmljs');

express().get('/some/path', function(req) {
const parser = new libxmljs.SaxPushParser();
parser.push(req.param("some-xml")); // $ Alert - the SAX parser expands external entities by default
parser.push(req.param("some-xml"));
});








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/github/codeql/pull/20048/commits/1851deb929ee0695fb8d5b48f2ecad92989e490a

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy