diff --git a/python-restclient/docs/MathModelResourceApi.md b/python-restclient/docs/MathModelResourceApi.md index fc1d5fcb53..fe3273775f 100644 --- a/python-restclient/docs/MathModelResourceApi.md +++ b/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) diff --git a/python-restclient/vcell_client/api/math_model_resource_api.py b/python-restclient/vcell_client/api/math_model_resource_api.py index b880b4bfb6..03a4c9b92f 100644 --- a/python-restclient/vcell_client/api/math_model_resource_api.py +++ b/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" } diff --git a/tools/openapi.yaml b/tools/openapi.yaml index 53d3cf4b94..f79a4cf928 100644 --- a/tools/openapi.yaml +++ b/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: diff --git a/vcell-core/src/main/java/cbit/util/xml/XmlUtil.java b/vcell-core/src/main/java/cbit/util/xml/XmlUtil.java index bff366ab0a..9ac1f206c5 100644 --- a/vcell-core/src/main/java/cbit/util/xml/XmlUtil.java +++ b/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; diff --git a/vcell-rest/src/main/java/org/vcell/restq/handlers/BioModelResource.java b/vcell-rest/src/main/java/org/vcell/restq/handlers/BioModelResource.java index f59cc95e05..79ecb491e7 100644 --- a/vcell-rest/src/main/java/org/vcell/restq/handlers/BioModelResource.java +++ b/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 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); } } diff --git a/vcell-rest/src/main/java/org/vcell/restq/handlers/MathModelResource.java b/vcell-rest/src/main/java/org/vcell/restq/handlers/MathModelResource.java index 49b0dff59e..da1dc2aaf7 100644 --- a/vcell-rest/src/main/java/org/vcell/restq/handlers/MathModelResource.java +++ b/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,6 +59,7 @@ 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); @@ -66,6 +67,8 @@ public String getMathModel(@PathParam("id") String id) throws DataAccessWebExcep 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 newName, @Parameter(name = "simNames", required = false, description = BioModelResource.simsRequiringUpdatesDescription) - @QueryParam("simsRequiringUpdates") List simNames) throws DataAccessWebException, NotAuthenticatedWebException { + @QueryParam("simsRequiringUpdates") List 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); } } diff --git a/vcell-rest/src/test/java/org/vcell/restq/apiclient/BioModelApiTest.java b/vcell-rest/src/test/java/org/vcell/restq/apiclient/BioModelApiTest.java index 083c198c67..760fbf4fc6 100644 --- a/vcell-rest/src/test/java/org/vcell/restq/apiclient/BioModelApiTest.java +++ b/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")); + } + } } diff --git a/vcell-rest/src/test/resources/BadXML.xml b/vcell-rest/src/test/resources/BadXML.xml new file mode 100644 index 0000000000..ffd92d8223 --- /dev/null +++ b/vcell-rest/src/test/resources/BadXML.xml @@ -0,0 +1,10 @@ + + + ]> + + VCell + Ezequiel + Bad XML + This XML tries to reference an external entity! + \ No newline at end of file diff --git a/vcell-restclient/api/openapi.yaml b/vcell-restclient/api/openapi.yaml index 6264343a94..e7588bc3fd 100644 --- a/vcell-restclient/api/openapi.yaml +++ b/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: diff --git a/vcell-restclient/docs/MathModelResourceApi.md b/vcell-restclient/docs/MathModelResourceApi.md index 779d1972cb..6689bbca97 100644 --- a/vcell-restclient/docs/MathModelResourceApi.md +++ b/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 | - |