Clean XML

python-restclient/docs/MathModelResourceApi.md

@@ -352,6 +352,7 @@ Name | Type | Description  | Notes
 **200** | OK |  -  |
 **401** | Not Authorized |  -  |
 **403** | Not Allowed |  -  |
+**422** | Unprocessable content submitted |  -  |
 **500** | Data Access Exception |  -  |
 
 [[Back to top]](#) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to Model list]](../README.md#documentation-for-models) [[Back to README]](../README.md)

python-restclient/vcell_client/api/math_model_resource_api.py

@@ -1179,6 +1179,7 @@ def save_math_model(
             '200': "str",
             '401': "VCellHTTPError",
             '403': None,
+            '422': "VCellHTTPError",
             '500': "VCellHTTPError"
 
         }
@@ -1257,6 +1258,7 @@ def save_math_model_with_http_info(
             '200': "str",
             '401': "VCellHTTPError",
             '403': None,
+            '422': "VCellHTTPError",
             '500': "VCellHTTPError"
 
         }
@@ -1335,6 +1337,7 @@ def save_math_model_without_preload_content(
             '200': "str",
             '401': "VCellHTTPError",
             '403': None,
+            '422': "VCellHTTPError",
             '500': "VCellHTTPError"
 
         }

tools/openapi.yaml

@@ -907,6 +907,12 @@ paths:
                 $ref: '#/components/schemas/VCellHTTPError'
         "403":
           description: Not Allowed
+        "422":
+          description: Unprocessable content submitted
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/VCellHTTPError'
         "500":
           description: Data Access Exception
           content:

vcell-core/src/main/java/cbit/util/xml/XmlUtil.java

@@ -127,17 +127,34 @@ public static void writeXMLStringToFile(String xmlString, String filename, boole
 	}
 
 
+	/**
+	 * Throws exception if the xml contains entities not allowed.
+	 * @param xml
+	 * @throws IOException
+	 * @throws JDOMException
+	 */
+	public static void vetXMLForMaliciousEntities(String xml) throws IOException, JDOMException {
+		SAXBuilder builder = new SAXBuilder();
+		GenericXMLErrorHandler errorHandler = new GenericXMLErrorHandler();
+
+		builder.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); // https://semgrep.dev/docs/cheat-sheets/java-xxe
+		builder.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+
+		builder.build(new StringReader(xml));
+	}
+
+
 //useful for the translators.
 	public static Document readXML(Reader reader, String schemaLocation, String parserClass, String schemaLocationPropName) throws RuntimeException {
 
-		SAXBuilder builder = null; 
+		SAXBuilder builder = null;
 		Document sDoc = null;
 		GenericXMLErrorHandler errorHandler = new GenericXMLErrorHandler();
 		try {
 			if (schemaLocation != null && schemaLocation.length() > 0) {           //ignores the parserClass, since xerces is the only validating parser we have
 		  		builder = new SAXBuilder("org.apache.xerces.parsers.SAXParser", true);
-		  		builder.setFeature("https://xml.org/sax/features/validation", true);
-	      		builder.setFeature("https://apache.org/xml/features/validation/schema", true);
+		  		builder.setFeature("http://xml.org/sax/features/validation", true);
+	      		builder.setFeature("http://apache.org/xml/features/validation/schema", true);
 	      		builder.setErrorHandler(errorHandler);
 				builder.setProperty(schemaLocationPropName, schemaLocation);
 			} else {                                                              //ignore schemaLocationPropName
@@ -149,6 +166,7 @@ public static Document readXML(Reader reader, String schemaLocation, String pars
 				builder.setErrorHandler(errorHandler);
 			}
 			builder.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); // https://semgrep.dev/docs/cheat-sheets/java-xxe
+			builder.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
 			sDoc = builder.build(reader);
 			// ----- Element root = null;
 	  		// ----- root = sDoc.getRootElement();
@@ -175,6 +193,8 @@ public static Document readXML(File file) throws RuntimeException {
 		Document sDoc = null;
 		GenericXMLErrorHandler errorHandler = new GenericXMLErrorHandler();
   		builder.setErrorHandler(errorHandler);
+		builder.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); // https://semgrep.dev/docs/cheat-sheets/java-xxe
+		builder.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
 		try {
 	  		sDoc = builder.build(file);
 			// Element root = null;
@@ -202,6 +222,8 @@ public static Document readXML(InputStream inputStream) throws RuntimeException
 		Document sDoc = null;
 		GenericXMLErrorHandler errorHandler = new GenericXMLErrorHandler();
   		builder.setErrorHandler(errorHandler);
+		builder.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); // https://semgrep.dev/docs/cheat-sheets/java-xxe
+		builder.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
 		try {
 	  		sDoc = builder.build(inputStream);
 			// Element root = null;

vcell-rest/src/main/java/org/vcell/restq/handlers/BioModelResource.java

@@ -1,5 +1,6 @@
 package org.vcell.restq.handlers;
 
+import cbit.util.xml.XmlUtil;
 import cbit.vcell.modeldb.BioModelRep;
 import cbit.vcell.xml.XmlHelper;
 import cbit.vcell.xml.XmlParseException;
@@ -9,11 +10,9 @@
 import jakarta.ws.rs.*;
 import jakarta.ws.rs.core.MediaType;
 import org.eclipse.microprofile.openapi.annotations.Operation;
-import org.eclipse.microprofile.openapi.annotations.media.Content;
-import org.eclipse.microprofile.openapi.annotations.media.Schema;
 import org.eclipse.microprofile.openapi.annotations.parameters.Parameter;
 import org.eclipse.microprofile.openapi.annotations.parameters.RequestBody;
-import org.jboss.resteasy.reactive.Separator;
+import org.jdom2.JDOMException;
 import org.vcell.restq.Main;
 import org.vcell.restq.errors.exceptions.*;
 import org.vcell.restq.models.BioModel;
@@ -24,6 +23,7 @@
 import org.vcell.util.PermissionException;
 import org.vcell.util.document.*;
 
+import java.io.IOException;
 import java.util.List;
 import java.util.Optional;
 
@@ -110,13 +110,17 @@ public String getBioModelVCML(@PathParam("bioModelID") String bioModelID) throws
             vcellUser = Main.DUMMY_USER;
         }
         try {
-            return bioModelRestService.getBioModel(vcellUser, new KeyValue(bioModelID));
+            String vcml = bioModelRestService.getBioModel(vcellUser, new KeyValue(bioModelID));
+            XmlUtil.vetXMLForMaliciousEntities(vcml);
+            return vcml;
         }catch (ObjectNotFoundException e) {
             throw new NotFoundWebException(e.getMessage(), e);
         } catch (PermissionException e){
             throw new PermissionWebException(e.getMessage(), e);
         } catch (DataAccessException e) {
             throw new DataAccessWebException(e.getMessage(), e);
+        } catch (IOException | JDOMException e) {
+            throw new RuntimeWebException("The BioModel you are trying to retrieve seems to be malformed. Please contact VCell support with the BioModel ID: " + bioModelID, e);
         }
     }
 
@@ -179,12 +183,15 @@ public String save(@RequestBody(name = "bioModelVCML", required = true, descript
                        @QueryParam("simsRequiringUpdates") @Parameter(required = false, allowEmptyValue = true, description = simsRequiringUpdatesDescription) List<String> simNames) throws DataAccessWebException, UnprocessableContentWebException, NotAuthenticatedWebException {
         User user = userRestService.getUserFromIdentity(securityIdentity);
         try {
+            XmlUtil.vetXMLForMaliciousEntities(bioModelVCML);
             cbit.vcell.biomodel.BioModel savedBioModel = bioModelRestService.save(user, bioModelVCML,
                     newName.orElse(null), simNames.toArray(new String[0]));
-            return XmlHelper.bioModelToXML(savedBioModel);
+            String result = XmlHelper.bioModelToXML(savedBioModel);
+            XmlUtil.vetXMLForMaliciousEntities(result);
+            return result;
         } catch (DataAccessException e) {
             throw new DataAccessWebException(e.getMessage(), e);
-        } catch (XmlParseException e){
+        } catch (XmlParseException | IOException | JDOMException e){
             throw new UnprocessableContentWebException(e.getMessage(), e);
         }
     }

vcell-rest/src/main/java/org/vcell/restq/handlers/MathModelResource.java

@@ -1,6 +1,7 @@
 package org.vcell.restq.handlers;
 
 
+import cbit.util.xml.XmlUtil;
 import io.quarkus.security.identity.SecurityIdentity;
 import jakarta.annotation.security.RolesAllowed;
 import jakarta.inject.Inject;
@@ -9,10 +10,8 @@
 import org.eclipse.microprofile.openapi.annotations.Operation;
 import org.eclipse.microprofile.openapi.annotations.parameters.Parameter;
 import org.eclipse.microprofile.openapi.annotations.parameters.RequestBody;
-import org.vcell.restq.errors.exceptions.DataAccessWebException;
-import org.vcell.restq.errors.exceptions.NotAuthenticatedWebException;
-import org.vcell.restq.errors.exceptions.NotFoundWebException;
-import org.vcell.restq.errors.exceptions.PermissionWebException;
+import org.jdom2.JDOMException;
+import org.vcell.restq.errors.exceptions.*;
 import org.vcell.restq.services.MathModelService;
 import org.vcell.restq.services.UserRestService;
 import org.vcell.util.BigString;
@@ -21,6 +20,7 @@
 import org.vcell.util.PermissionException;
 import org.vcell.util.document.*;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
@@ -59,13 +59,16 @@ public String getMathModel(@PathParam("id") String id) throws DataAccessWebExcep
         User user = userRestService.getUserOrAnonymousFromIdentity(securityIdentity);
         try{
             BigString result = mathModelService.getMathModelVCML(user, new KeyValue(id));
+            XmlUtil.vetXMLForMaliciousEntities(result.toString());
             return result.toString();
         } catch (ObjectNotFoundException e){
             throw new NotFoundWebException(e.getMessage(), e);
         } catch (PermissionException e){
             throw new PermissionWebException(e.getMessage(), e);
         } catch (DataAccessException e) {
             throw new DataAccessWebException(e.getMessage(), e);
+        } catch (IOException | JDOMException e) {
+            throw new RuntimeWebException("The MathModel you are trying to retrieve seems to be malformed, please contact VCell support with the Math Model ID: " + id, e);
         }
     }
 
@@ -124,13 +127,17 @@ public String save(@RequestBody(name = "mathModelVCML", required = true) String
                        @Parameter(name = "newName", required = false, description = "Name to save new MathModel under. Leave blank if re-saving existing MathModel.")
                        @QueryParam("newName") Optional<String> newName,
                        @Parameter(name = "simNames", required = false, description = BioModelResource.simsRequiringUpdatesDescription)
-                       @QueryParam("simsRequiringUpdates") List<String> simNames) throws DataAccessWebException, NotAuthenticatedWebException {
+                       @QueryParam("simsRequiringUpdates") List<String> simNames) throws DataAccessWebException, NotAuthenticatedWebException, UnprocessableContentWebException {
         User user = userRestService.getUserFromIdentity(securityIdentity);
         try{
+            XmlUtil.vetXMLForMaliciousEntities(mathModelVCML);
             BigString result = mathModelService.saveModel(user, new BigString(mathModelVCML), newName.orElse(null), simNames.toArray(new String[0]));
+            XmlUtil.vetXMLForMaliciousEntities(result.toString()); // partial saves might include already saved XML
             return result.toString();
         } catch (DataAccessException e) {
             throw new DataAccessWebException(e.getMessage(), e);
+        } catch (IOException | JDOMException e) {
+            throw new UnprocessableContentWebException(e.getMessage(), e);
         }
     }
 

vcell-rest/src/test/java/org/vcell/restq/apiclient/BioModelApiTest.java

@@ -28,6 +28,7 @@
 import org.vcell.restq.config.CDIVCellConfigProvider;
 import org.vcell.restq.db.AgroalConnectionFactory;
 import org.vcell.restq.errors.exceptions.DataAccessWebException;
+import org.vcell.restq.errors.exceptions.UnprocessableContentWebException;
 import org.vcell.util.DataAccessException;
 import org.vcell.util.ObjectNotFoundException;
 import org.vcell.util.document.BioModelChildSummary;
@@ -36,6 +37,7 @@
 
 import java.beans.PropertyVetoException;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.sql.SQLException;
 import java.util.ArrayList;
 
@@ -166,4 +168,17 @@ public void testGetBioModelContext() throws PropertyVetoException, XmlParseExcep
         Assertions.assertEquals(BioModelChildSummary.MathType.Deterministic, info.getBioModelChildSummary().getAppTypes()[0]);
         Assertions.assertEquals(new ArrayList<>(){{add("non-spatial ODE");}}, context.getSummary().getSimulationContextNames());
     }
+
+    @Test
+    public void testBadXML() throws IOException, ApiException {
+        BioModelResourceApi bioModelResourceApi = new BioModelResourceApi(aliceAPIClient);
+        String badXML = Files.readString(TestEndpointUtils.getResourceFile("/BadXML.xml").toPath());
+        try{
+            bioModelResourceApi.saveBioModel(badXML, null, null);
+            Assertions.fail();
+        } catch (ApiException e){
+            Assertions.assertEquals(UnprocessableContentWebException.HTTP_CODE, e.getCode());
+            Assertions.assertTrue(e.getMessage().contains("DOCTYPE is disallowed"));
+        }
+    }
 }

vcell-rest/src/test/resources/BadXML.xml

@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE note [
+        <!ENTITY company SYSTEM "https://vcell.org">
+        ]>
+<note>
+    <to>VCell</to>
+    <from>Ezequiel</from>
+    <heading>Bad XML</heading>
+    <body>This XML tries to reference an external entity!</body>
+</note>

vcell-restclient/api/openapi.yaml

@@ -931,6 +931,12 @@ paths:
           description: Not Authorized
         "403":
           description: Not Allowed
+        "422":
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/VCellHTTPError'
+          description: Unprocessable content submitted
         "500":
           content:
             application/json:

vcell-restclient/docs/MathModelResourceApi.md

@@ -643,6 +643,7 @@ public class Example {
 | **200** | OK |  -  |
 | **401** | Not Authorized |  -  |
 | **403** | Not Allowed |  -  |
+| **422** | Unprocessable content submitted |  -  |
 | **500** | Data Access Exception |  -  |
 
 ## saveMathModelWithHttpInfo
@@ -718,5 +719,6 @@ ApiResponse<**String**>
 | **200** | OK |  -  |
 | **401** | Not Authorized |  -  |
 | **403** | Not Allowed |  -  |
+| **422** | Unprocessable content submitted |  -  |
 | **500** | Data Access Exception |  -  |