Skip to content

Commit e78ae1a

Browse files
dizzzzdizzzz
authored
Merge pull request #2243 from adamretter/hotfix/close-rest-xxe
Secure the XMLReader processing
2 parents 581e787 + b210f9f commit e78ae1a

File tree

37 files changed

+269
-222
lines changed

37 files changed

+269
-222
lines changed

‎extensions/debuggee/src/org/exist/debugger/Utils.java‎

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,6 @@
2424
import java.io.IOException;
2525
import java.io.StringReader;
2626

27-
import javax.xml.parsers.SAXParser;
28-
import javax.xml.parsers.SAXParserFactory;
29-
3027
import org.exist.Namespaces;
3128
import org.exist.dom.memtree.NodeImpl;
3229
import org.exist.dom.memtree.SAXAdapter;
@@ -44,28 +41,30 @@ public class Utils {
4441
public static NodeImpl nodeFromString(XQueryContext context, String source) throws IOException {
4542
SAXAdapter adapter = new SAXAdapter(context);
4643

47-
SAXParserFactory factory = SAXParserFactory.newInstance();
48-
factory.setNamespaceAware(true);
49-
50-
XMLReader xr;
51-
try {
52-
SAXParser parser = factory.newSAXParser();
44+
final XMLReaderPool parserPool = context.getBroker().getBrokerPool().getParserPool();
45+
XMLReader xr = null;
46+
try {
47+
try {
48+
xr = parserPool.borrowXMLReader();
49+
xr.setContentHandler(adapter);
50+
xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter);
51+
52+
} catch (Exception e) {
53+
throw new IOException(e);
54+
}
5355

54-
xr = parser.getXMLReader();
55-
xr.setContentHandler(adapter);
56-
xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter);
56+
try {
57+
InputSource src = new InputSource(new StringReader(source));
58+
xr.parse(src);
5759

58-
} catch (Exception e) {
59-
throw new IOException(e);
60-
}
61-
62-
try {
63-
InputSource src = new InputSource(new StringReader(source));
64-
xr.parse(src);
65-
66-
return (NodeImpl) adapter.getDocument();
67-
} catch (SAXException e) {
68-
throw new IOException(e);
60+
return (NodeImpl) adapter.getDocument();
61+
} catch (SAXException e) {
62+
throw new IOException(e);
63+
}
64+
} finally {
65+
if (xr != null) {
66+
parserPool.returnXMLReader(xr);
67+
}
6968
}
7069
}
7170
}

‎extensions/debuggee/src/org/exist/debugger/dbgp/ResponseImpl.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.exist.debugger.DebuggerImpl;
3333
import org.exist.debugger.Response;
3434
import org.exist.dom.memtree.SAXAdapter;
35+
import org.exist.util.ExistSAXParserFactory;
3536
import org.w3c.dom.Element;
3637
import org.w3c.dom.Node;
3738
import org.w3c.dom.NodeList;
@@ -53,7 +54,7 @@ public class ResponseImpl implements Response {
5354
public ResponseImpl(IoSession session, InputStream inputStream) {
5455
this.session = session;
5556

56-
SAXParserFactory factory = SAXParserFactory.newInstance();
57+
SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory();
5758
factory.setNamespaceAware(true);
5859
InputSource src = new InputSource(inputStream);
5960
SAXParser parser;

‎extensions/indexes/spatial/src/org/exist/indexing/spatial/AbstractGMLJDBCIndexWorker.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,6 @@
7878
import org.xml.sax.helpers.XMLFilterImpl;
7979

8080
import javax.xml.parsers.ParserConfigurationException;
81-
import javax.xml.parsers.SAXParser;
82-
import javax.xml.parsers.SAXParserFactory;
8381
import javax.xml.transform.TransformerException;
8482
import java.io.IOException;
8583
import java.io.StringReader;
@@ -642,15 +640,13 @@ public Element streamGeometryToElement(Geometry geometry, String srsName, Receiv
642640
gmlString = gmlTransformer.transform(geometry);
643641
} catch (TransformerException e) {
644642
throw new SpatialIndexException(e);
645-
}
643+
}
646644

645+
final XMLReaderPool parserPool = pool.getParserPool();
646+
XMLReader reader = null;
647647
try {
648-
//Copied from org.exist.xquery.functions.request.getData
649-
SAXParserFactory factory = SAXParserFactory.newInstance();
650-
factory.setNamespaceAware(true);
651648
InputSource src = new InputSource(new StringReader(gmlString));
652-
SAXParser parser = factory.newSAXParser();
653-
XMLReader reader = parser.getXMLReader();
649+
reader = parserPool.borrowXMLReader();
654650
reader.setContentHandler((ContentHandler)receiver);
655651
reader.parse(src);
656652
Document doc = receiver.getDocument();
@@ -660,7 +656,11 @@ public Element streamGeometryToElement(Geometry geometry, String srsName, Receiv
660656
} catch (SAXException e) {
661657
throw new SpatialIndexException(e);
662658
} catch (IOException e) {
663-
throw new SpatialIndexException(e);
659+
throw new SpatialIndexException(e);
660+
} finally {
661+
if (reader != null) {
662+
parserPool.returnXMLReader(reader);
663+
}
664664
}
665665
}
666666

‎extensions/indexes/spatial/test/src/org/exist/indexing/spatial/GMLIndexTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import org.exist.security.PermissionDeniedException;
4747
import org.exist.storage.BrokerPool;
4848
import org.exist.storage.DBBroker;
49+
import org.exist.util.ExistSAXParserFactory;
4950
import org.exist.xmldb.IndexQueryService;
5051
import org.exist.xmldb.XmldbURI;
5152
import org.exist.xquery.XPathException;
@@ -203,7 +204,7 @@ public void testLowLevelSearch() throws EXistException, SAXException, ParserConf
203204
AbstractGMLJDBCIndexWorker indexWorker = (AbstractGMLJDBCIndexWorker) broker.getIndexController().getWorkerByIndexId(AbstractGMLJDBCIndex.ID);
204205
//Unplugged
205206
if (indexWorker != null) {
206-
SAXParserFactory factory = SAXParserFactory.newInstance();
207+
SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory();
207208
factory.setNamespaceAware(true);
208209
InputSource src = new InputSource(new StringReader(IN_MEMORY_GML));
209210
SAXParser parser = factory.newSAXParser();

‎extensions/modules/src/org/exist/xquery/modules/sql/ExecuteFunction.java‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.apache.logging.log4j.LogManager;
2626
import org.apache.logging.log4j.Logger;
2727

28+
import org.exist.util.XMLReaderPool;
2829
import org.exist.util.io.FastByteArrayOutputStream;
2930
import org.w3c.dom.Element;
3031
import org.w3c.dom.Node;
@@ -60,8 +61,6 @@
6061
import java.sql.Statement;
6162
import java.sql.Timestamp;
6263
import java.sql.Types;
63-
import javax.xml.parsers.SAXParser;
64-
import javax.xml.parsers.SAXParserFactory;
6564
import org.exist.dom.memtree.AppendingSAXAdapter;
6665
import org.exist.dom.memtree.ReferenceNode;
6766
import org.exist.dom.memtree.SAXAdapter;
@@ -263,17 +262,21 @@ public ExecuteFunction( XQueryContext context, FunctionSignature signature )
263262
// Add a null indicator attribute if the value was SQL Null
264263
builder.addAttribute( new QName( "null", SQLModule.NAMESPACE_URI, SQLModule.PREFIX ), "true" );
265264
} else {
266-
267-
SAXParserFactory factory = SAXParserFactory.newInstance();
268-
factory.setNamespaceAware(true);
269265
InputSource src = new InputSource(sqlXml.getCharacterStream());
270-
SAXParser parser = factory.newSAXParser();
271-
XMLReader xr = parser.getXMLReader();
272-
273-
SAXAdapter adapter = new AppendingSAXAdapter(builder);
274-
xr.setContentHandler(adapter);
275-
xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter);
276-
xr.parse(src);
266+
final XMLReaderPool parserPool = context.getBroker().getBrokerPool().getParserPool();
267+
XMLReader reader = null;
268+
try {
269+
reader = parserPool.borrowXMLReader();
270+
271+
SAXAdapter adapter = new AppendingSAXAdapter(builder);
272+
reader.setContentHandler(adapter);
273+
reader.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter);
274+
reader.parse(src);
275+
} finally {
276+
if (reader != null) {
277+
parserPool.returnXMLReader(reader);
278+
}
279+
}
277280
}
278281
} catch(Exception e) {
279282
throw new XPathException("Could not parse column of type SQLXML: " + e.getMessage(), e);

‎src/org/exist/backup/AbstractBackupDescriptor.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
package org.exist.backup;
2424

25+
import org.exist.util.XMLReaderPool;
2526
import org.xml.sax.ContentHandler;
2627
import org.xml.sax.SAXException;
2728
import org.xml.sax.XMLReader;
@@ -34,10 +35,6 @@
3435
import java.util.Date;
3536
import java.util.Properties;
3637

37-
import javax.xml.parsers.ParserConfigurationException;
38-
import javax.xml.parsers.SAXParser;
39-
import javax.xml.parsers.SAXParserFactory;
40-
4138

4239
public abstract class AbstractBackupDescriptor implements BackupDescriptor
4340
{
@@ -74,15 +71,18 @@ public boolean before( long timestamp )
7471
return( timestamp > getDate().getTime() );
7572
}
7673

77-
78-
public void parse( ContentHandler handler ) throws IOException, SAXException, ParserConfigurationException
74+
@Override
75+
public void parse(final XMLReaderPool parserPool, final ContentHandler handler ) throws IOException, SAXException
7976
{
80-
final SAXParserFactory saxFactory = SAXParserFactory.newInstance();
81-
saxFactory.setNamespaceAware( true );
82-
saxFactory.setValidating( false );
83-
final SAXParser sax = saxFactory.newSAXParser();
84-
final XMLReader reader = sax.getXMLReader();
85-
reader.setContentHandler( handler );
86-
reader.parse( getInputSource() );
77+
XMLReader reader = null;
78+
try {
79+
reader = parserPool.borrowXMLReader();
80+
reader.setContentHandler(handler);
81+
reader.parse(getInputSource());
82+
} finally {
83+
if (reader != null) {
84+
parserPool.returnXMLReader(reader);
85+
}
86+
}
8787
}
8888
}

‎src/org/exist/backup/BackupDescriptor.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
*/
2222
package org.exist.backup;
2323

24+
import org.exist.util.XMLReaderPool;
2425
import org.xml.sax.ContentHandler;
2526
import org.xml.sax.SAXException;
2627

@@ -32,8 +33,6 @@
3233
import java.util.Date;
3334
import java.util.Properties;
3435

35-
import javax.xml.parsers.ParserConfigurationException;
36-
3736

3837
public interface BackupDescriptor
3938
{
@@ -86,7 +85,8 @@ public interface BackupDescriptor
8685
boolean before( long timestamp );
8786

8887

89-
void parse( ContentHandler handler ) throws IOException, SAXException, ParserConfigurationException;
88+
void parse(XMLReaderPool parserPool, ContentHandler handler)
89+
throws IOException, SAXException;
9090

9191
Path getRepoBackup() throws IOException;
9292
}

‎src/org/exist/backup/Restore.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.exist.security.Account;
3838
import org.exist.security.SecurityManager;
3939
import org.exist.util.EXistInputSource;
40+
import org.exist.util.ExistSAXParserFactory;
4041
import org.exist.util.FileUtils;
4142
import org.exist.xmldb.UserManagementService;
4243
import org.exist.xmldb.XmldbURI;
@@ -67,7 +68,7 @@ public void restore(RestoreListener listener, String username, String password,
6768
//get the backup descriptors, can be more than one if it was an incremental backup
6869
final Deque<BackupDescriptor> descriptors = getBackupDescriptors(f);
6970

70-
final SAXParserFactory saxFactory = SAXParserFactory.newInstance();
71+
final SAXParserFactory saxFactory = ExistSAXParserFactory.getSAXParserFactory();
7172
saxFactory.setNamespaceAware(true);
7273
saxFactory.setValidating(false);
7374
final SAXParser sax = saxFactory.newSAXParser();

‎src/org/exist/backup/SystemExport.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ private void export(BackupHandler bh, Collection current, BackupWriter output, D
462462
final CheckDeletedHandler check = new CheckDeletedHandler(current, serializer);
463463

464464
try {
465-
prevBackup.parse(check);
465+
prevBackup.parse(broker.getBrokerPool().getParserPool(), check);
466466
} catch (final Exception e) {
467467
LOG.error("Caught exception while trying to parse previous backup descriptor: " + prevBackup.getSymbolicPath(), e);
468468
}

‎src/org/exist/backup/SystemImport.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@
2929
import java.util.Deque;
3030
import java.util.Properties;
3131
import javax.xml.parsers.ParserConfigurationException;
32-
import javax.xml.parsers.SAXParser;
33-
import javax.xml.parsers.SAXParserFactory;
3432

3533
import org.apache.logging.log4j.LogManager;
3634
import org.apache.logging.log4j.Logger;
@@ -44,6 +42,7 @@
4442
import org.exist.storage.DBBroker;
4543
import org.exist.util.EXistInputSource;
4644
import org.exist.util.FileUtils;
45+
import org.exist.util.XMLReaderPool;
4746
import org.exist.xmldb.XmldbURI;
4847
import org.xml.sax.SAXException;
4948
import org.xml.sax.XMLReader;
@@ -74,15 +73,13 @@ public void restore(RestoreListener listener, String username, Object credential
7473

7574
//get the backup descriptors, can be more than one if it was an incremental backup
7675
final Deque<BackupDescriptor> descriptors = getBackupDescriptors(f);
77-
78-
final SAXParserFactory saxFactory = SAXParserFactory.newInstance();
79-
saxFactory.setNamespaceAware(true);
80-
saxFactory.setValidating(false);
81-
final SAXParser sax = saxFactory.newSAXParser();
82-
final XMLReader reader = sax.getXMLReader();
83-
76+
77+
final XMLReaderPool parserPool = broker.getBrokerPool().getParserPool();
78+
XMLReader reader = null;
8479
try {
85-
listener.restoreStarting();
80+
reader = parserPool.borrowXMLReader();
81+
82+
listener.restoreStarting();
8683

8784
while(!descriptors.isEmpty()) {
8885
final BackupDescriptor descriptor = descriptors.pop();
@@ -96,6 +93,10 @@ public void restore(RestoreListener listener, String username, Object credential
9693
}
9794
} finally {
9895
listener.restoreFinished();
96+
97+
if (reader != null) {
98+
parserPool.returnXMLReader(reader);
99+
}
99100
}
100101
}
101102
}

0 commit comments

Comments
 (0)