Skip to content

Commit

Permalink
[bugfix] Secure the XMLReader processing
Browse files Browse the repository at this point in the history 
Closes #2180
  • Loading branch information
@adamretter
adamretter committed Oct 30, 2018
1 parent cb5df6c commit 1c3f0ae
Show file tree
Hide file tree
Showing 35 changed files with 266 additions and 219 deletions.
45 changes: 22 additions & 23 deletions extensions/debuggee/src/org/exist/debugger/Utils.java
View file
Expand Up @@ -24,9 +24,6 @@
import java.io.IOException;
import java.io.StringReader;

import javax.xml.parsers.SAXParser;
import javax.xml.parsers.SAXParserFactory;

import org.exist.Namespaces;
import org.exist.dom.memtree.NodeImpl;
import org.exist.dom.memtree.SAXAdapter;
Expand All @@ -44,28 +41,30 @@ public class Utils {
public static NodeImpl nodeFromString(XQueryContext context, String source) throws IOException {
SAXAdapter adapter = new SAXAdapter(context);

SAXParserFactory factory = SAXParserFactory.newInstance();
factory.setNamespaceAware(true);

XMLReader xr;
try {
SAXParser parser = factory.newSAXParser();
final XMLReaderPool parserPool = context.getBroker().getBrokerPool().getParserPool();
XMLReader xr = null;
try {
try {
xr = parserPool.borrowXMLReader();
xr.setContentHandler(adapter);
xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter);

} catch (Exception e) {
throw new IOException(e);
}

xr = parser.getXMLReader();
xr.setContentHandler(adapter);
xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter);
try {
InputSource src = new InputSource(new StringReader(source));
xr.parse(src);

} catch (Exception e) {
throw new IOException(e);
}

try {
InputSource src = new InputSource(new StringReader(source));
xr.parse(src);

return (NodeImpl) adapter.getDocument();
} catch (SAXException e) {
throw new IOException(e);
return (NodeImpl) adapter.getDocument();
} catch (SAXException e) {
throw new IOException(e);
}
} finally {
if (xr != null) {
parserPool.returnXMLReader(xr);
}
}
}
}
3 changes: 2 additions & 1 deletion extensions/debuggee/src/org/exist/debugger/dbgp/ResponseImpl.java
View file
Expand Up @@ -32,6 +32,7 @@
import org.exist.debugger.DebuggerImpl;
import org.exist.debugger.Response;
import org.exist.dom.memtree.SAXAdapter;
import org.exist.util.ExistSAXParserFactory;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
Expand All @@ -53,7 +54,7 @@ public class ResponseImpl implements Response {
public ResponseImpl(IoSession session, InputStream inputStream) {
this.session = session;

SAXParserFactory factory = SAXParserFactory.newInstance();
SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory();
factory.setNamespaceAware(true);
InputSource src = new InputSource(inputStream);
SAXParser parser;
Expand Down
18 changes: 9 additions & 9 deletions extensions/indexes/spatial/src/org/exist/indexing/spatial/AbstractGMLJDBCIndexWorker.java
View file
Expand Up @@ -78,8 +78,6 @@
import org.xml.sax.helpers.XMLFilterImpl;

import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParser;
import javax.xml.parsers.SAXParserFactory;
import javax.xml.transform.TransformerException;
import java.io.IOException;
import java.io.StringReader;
Expand Down Expand Up @@ -642,15 +640,13 @@ public Element streamGeometryToElement(Geometry geometry, String srsName, Receiv
gmlString = gmlTransformer.transform(geometry);
} catch (TransformerException e) {
throw new SpatialIndexException(e);
}
}

final XMLReaderPool parserPool = pool.getParserPool();
XMLReader reader = null;
try {
//Copied from org.exist.xquery.functions.request.getData
SAXParserFactory factory = SAXParserFactory.newInstance();
factory.setNamespaceAware(true);
InputSource src = new InputSource(new StringReader(gmlString));
SAXParser parser = factory.newSAXParser();
XMLReader reader = parser.getXMLReader();
reader = parserPool.borrowXMLReader();
reader.setContentHandler((ContentHandler)receiver);
reader.parse(src);
Document doc = receiver.getDocument();
Expand All @@ -660,7 +656,11 @@ public Element streamGeometryToElement(Geometry geometry, String srsName, Receiv
} catch (SAXException e) {
throw new SpatialIndexException(e);
} catch (IOException e) {
throw new SpatialIndexException(e);
throw new SpatialIndexException(e);
} finally {
if (reader != null) {
parserPool.returnXMLReader(reader);
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion extensions/indexes/spatial/test/src/org/exist/indexing/spatial/GMLIndexTest.java
View file
Expand Up @@ -46,6 +46,7 @@
import org.exist.security.PermissionDeniedException;
import org.exist.storage.BrokerPool;
import org.exist.storage.DBBroker;
import org.exist.util.ExistSAXParserFactory;
import org.exist.xmldb.IndexQueryService;
import org.exist.xmldb.XmldbURI;
import org.exist.xquery.XPathException;
Expand Down Expand Up @@ -203,7 +204,7 @@ public void testLowLevelSearch() throws EXistException, SAXException, ParserConf
AbstractGMLJDBCIndexWorker indexWorker = (AbstractGMLJDBCIndexWorker) broker.getIndexController().getWorkerByIndexId(AbstractGMLJDBCIndex.ID);
//Unplugged
if (indexWorker != null) {
SAXParserFactory factory = SAXParserFactory.newInstance();
SAXParserFactory factory = ExistSAXParserFactory.getSAXParserFactory();
factory.setNamespaceAware(true);
InputSource src = new InputSource(new StringReader(IN_MEMORY_GML));
SAXParser parser = factory.newSAXParser();
Expand Down
27 changes: 15 additions & 12 deletions extensions/modules/src/org/exist/xquery/modules/sql/ExecuteFunction.java
View file
Expand Up @@ -25,6 +25,7 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import org.exist.util.XMLReaderPool;
import org.exist.util.io.FastByteArrayOutputStream;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
Expand Down Expand Up @@ -60,8 +61,6 @@
import java.sql.Statement;
import java.sql.Timestamp;
import java.sql.Types;
import javax.xml.parsers.SAXParser;
import javax.xml.parsers.SAXParserFactory;
import org.exist.dom.memtree.AppendingSAXAdapter;
import org.exist.dom.memtree.ReferenceNode;
import org.exist.dom.memtree.SAXAdapter;
Expand Down Expand Up @@ -263,17 +262,21 @@ public ExecuteFunction( XQueryContext context, FunctionSignature signature )
// Add a null indicator attribute if the value was SQL Null
builder.addAttribute( new QName( "null", SQLModule.NAMESPACE_URI, SQLModule.PREFIX ), "true" );
} else {

SAXParserFactory factory = SAXParserFactory.newInstance();
factory.setNamespaceAware(true);
InputSource src = new InputSource(sqlXml.getCharacterStream());
SAXParser parser = factory.newSAXParser();
XMLReader xr = parser.getXMLReader();

SAXAdapter adapter = new AppendingSAXAdapter(builder);
xr.setContentHandler(adapter);
xr.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter);
xr.parse(src);
final XMLReaderPool parserPool = context.getBroker().getBrokerPool().getParserPool();
XMLReader reader = null;
try {
reader = parserPool.borrowXMLReader();

SAXAdapter adapter = new AppendingSAXAdapter(builder);
reader.setContentHandler(adapter);
reader.setProperty(Namespaces.SAX_LEXICAL_HANDLER, adapter);
reader.parse(src);
} finally {
if (reader != null) {
parserPool.returnXMLReader(reader);
}
}
}
} catch(Exception e) {
throw new XPathException("Could not parse column of type SQLXML: " + e.getMessage(), e);
Expand Down
26 changes: 13 additions & 13 deletions src/org/exist/backup/AbstractBackupDescriptor.java
View file
Expand Up @@ -22,6 +22,7 @@

package org.exist.backup;

import org.exist.util.XMLReaderPool;
import org.xml.sax.ContentHandler;
import org.xml.sax.SAXException;
import org.xml.sax.XMLReader;
Expand All @@ -34,10 +35,6 @@
import java.util.Date;
import java.util.Properties;

import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParser;
import javax.xml.parsers.SAXParserFactory;


public abstract class AbstractBackupDescriptor implements BackupDescriptor
{
Expand Down Expand Up @@ -74,15 +71,18 @@ public boolean before( long timestamp )
return( timestamp > getDate().getTime() );
}


public void parse( ContentHandler handler ) throws IOException, SAXException, ParserConfigurationException
@Override
public void parse(final XMLReaderPool parserPool, final ContentHandler handler ) throws IOException, SAXException
{
final SAXParserFactory saxFactory = SAXParserFactory.newInstance();
saxFactory.setNamespaceAware( true );
saxFactory.setValidating( false );
final SAXParser sax = saxFactory.newSAXParser();
final XMLReader reader = sax.getXMLReader();
reader.setContentHandler( handler );
reader.parse( getInputSource() );
XMLReader reader = null;
try {
reader = parserPool.borrowXMLReader();
reader.setContentHandler(handler);
reader.parse(getInputSource());
} finally {
if (reader != null) {
parserPool.returnXMLReader(reader);
}
}
}
}
6 changes: 3 additions & 3 deletions src/org/exist/backup/BackupDescriptor.java
View file
Expand Up @@ -21,6 +21,7 @@
*/
package org.exist.backup;

import org.exist.util.XMLReaderPool;
import org.xml.sax.ContentHandler;
import org.xml.sax.SAXException;

Expand All @@ -32,8 +33,6 @@
import java.util.Date;
import java.util.Properties;

import javax.xml.parsers.ParserConfigurationException;


public interface BackupDescriptor
{
Expand Down Expand Up @@ -86,7 +85,8 @@ public interface BackupDescriptor
boolean before( long timestamp );


void parse( ContentHandler handler ) throws IOException, SAXException, ParserConfigurationException;
void parse(XMLReaderPool parserPool, ContentHandler handler)
throws IOException, SAXException;

Path getRepoBackup() throws IOException;
}
3 changes: 2 additions & 1 deletion src/org/exist/backup/Restore.java
View file
Expand Up @@ -37,6 +37,7 @@
import org.exist.security.Account;
import org.exist.security.SecurityManager;
import org.exist.util.EXistInputSource;
import org.exist.util.ExistSAXParserFactory;
import org.exist.util.FileUtils;
import org.exist.xmldb.UserManagementService;
import org.exist.xmldb.XmldbURI;
Expand Down Expand Up @@ -67,7 +68,7 @@ public void restore(RestoreListener listener, String username, String password,
//get the backup descriptors, can be more than one if it was an incremental backup
final Deque<BackupDescriptor> descriptors = getBackupDescriptors(f);

final SAXParserFactory saxFactory = SAXParserFactory.newInstance();
final SAXParserFactory saxFactory = ExistSAXParserFactory.getSAXParserFactory();
saxFactory.setNamespaceAware(true);
saxFactory.setValidating(false);
final SAXParser sax = saxFactory.newSAXParser();
Expand Down
2 changes: 1 addition & 1 deletion src/org/exist/backup/SystemExport.java
View file
Expand Up @@ -462,7 +462,7 @@ private void export(BackupHandler bh, Collection current, BackupWriter output, D
final CheckDeletedHandler check = new CheckDeletedHandler(current, serializer);

try {
prevBackup.parse(check);
prevBackup.parse(broker.getBrokerPool().getParserPool(), check);
} catch (final Exception e) {
LOG.error("Caught exception while trying to parse previous backup descriptor: " + prevBackup.getSymbolicPath(), e);
}
Expand Down
21 changes: 11 additions & 10 deletions src/org/exist/backup/SystemImport.java
View file
Expand Up @@ -29,8 +29,6 @@
import java.util.Deque;
import java.util.Properties;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParser;
import javax.xml.parsers.SAXParserFactory;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand All @@ -44,6 +42,7 @@
import org.exist.storage.DBBroker;
import org.exist.util.EXistInputSource;
import org.exist.util.FileUtils;
import org.exist.util.XMLReaderPool;
import org.exist.xmldb.XmldbURI;
import org.xml.sax.SAXException;
import org.xml.sax.XMLReader;
Expand Down Expand Up @@ -74,15 +73,13 @@ public void restore(RestoreListener listener, String username, Object credential

//get the backup descriptors, can be more than one if it was an incremental backup
final Deque<BackupDescriptor> descriptors = getBackupDescriptors(f);

final SAXParserFactory saxFactory = SAXParserFactory.newInstance();
saxFactory.setNamespaceAware(true);
saxFactory.setValidating(false);
final SAXParser sax = saxFactory.newSAXParser();
final XMLReader reader = sax.getXMLReader();


final XMLReaderPool parserPool = broker.getBrokerPool().getParserPool();
XMLReader reader = null;
try {
listener.restoreStarting();
reader = parserPool.borrowXMLReader();

listener.restoreStarting();

while(!descriptors.isEmpty()) {
final BackupDescriptor descriptor = descriptors.pop();
Expand All @@ -96,6 +93,10 @@ public void restore(RestoreListener listener, String username, Object credential
}
} finally {
listener.restoreFinished();

if (reader != null) {
parserPool.returnXMLReader(reader);
}
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/org/exist/backup/restore/RestoreHandler.java
View file
Expand Up @@ -37,6 +37,7 @@
import org.exist.security.ACLPermission.ACE_TARGET;
import org.exist.security.SecurityManager;
import org.exist.util.EXistInputSource;
import org.exist.util.ExistSAXParserFactory;
import org.exist.xmldb.EXistCollection;
import org.exist.xmldb.EXistCollectionManagementService;
import org.exist.xmldb.EXistResource;
Expand Down Expand Up @@ -65,7 +66,7 @@
public class RestoreHandler extends DefaultHandler {

private final static Logger LOG = LogManager.getLogger(RestoreHandler.class);
private final static SAXParserFactory saxFactory = SAXParserFactory.newInstance();
private final static SAXParserFactory saxFactory = ExistSAXParserFactory.getSAXParserFactory();
static {
saxFactory.setNamespaceAware(true);
saxFactory.setValidating(false);
Expand Down

0 comments on commit 1c3f0ae

Please sign in to comment.