Updated docs for XXE

Rolly-M · Aug 6, 2018 · e6285d3 · e6285d3
1 parent f62ebd1
commit e6285d3
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 14 deletions.
diff --git a/docs/resources/xxe1.png b/docs/resources/xxe1.png
diff --git a/docs/resources/xxe2.png b/docs/resources/xxe2.png
diff --git a/docs/solution/a4-xxe.md b/docs/solution/a4-xxe.md
@@ -1,41 +1,62 @@
 # XML External Entities
 
-## XXE in XYZ
+The `Bulk Import` feature at http://127.0.0.1:9090/app/bulkproducts is vulnerable to XML External Entity attack.
+
+![xxe1](/resources/xxe1.png)
+
+This can be easily exploited by supplying an input like the one below
+
+```xml
+<!DOCTYPE foo [<!ELEMENT foo ANY >
+<!ENTITY bar SYSTEM "file:///etc/passwd" >]>
+<products>
+   <product>
+      <name>Playstation 4</name>
+      <code>274</code>
+      <tags>gaming console</tags>
+      <description>&bar;</description>
+   </product>
+</products>
+```
 
-There is a SQL Injection in `User Search` feature at the following URL  
+The resulting product's description will have the contents of `/etc/passwd`
 
-http://127.0.0.1:9090/app/usersearch
+![xxe2](/resources/xxe2.png)
 
 **Vulnerable Code snippet**
 
 *core/appHandler.js*
-```         
+```
 ...
-
+module.exports.bulkProducts =  function(req, res) {
+	if (req.files.products && req.files.products.mimetype=='text/xml'){
+		var products = libxmljs.parseXmlString(req.files.products.data.toString('utf8'), {noent:true,noblanks:true})
 ...
 ```
+
 **Solution**
 
+The XML parsing library used is `libxmljs` which allows for parsing external entities. We can disable parsing of external entities by modifying the flag value `noent` to `false`.
 
 *core/appHandler.js*
 ```
 ...
-
+module.exports.bulkProducts =  function(req, res) {
+	if (req.files.products && req.files.products.mimetype=='text/xml'){
+		var products = libxmljs.parseXmlString(req.files.products.data.toString('utf8'), {noent:false,noblanks:true})
 ...
 ```
 
-But it is recommended to explicitly validate/sanitize inputs
-
 **Fixes**
 
-Implemented in the following files
+Implemented in the following file
 
 - *core/appHandler.js*
 
 **Recommendation**
 
-- Validate Input before processing
-- Sanitize Input before storing
+- Ensure that External entity parsing is disabled
+- If parsing is absoutely required, then validate the data before parsing
 
 **Reference**
 

diff --git a/docs/solution/a8-insecure-deserialization.md b/docs/solution/a8-insecure-deserialization.md
@@ -1,8 +1,6 @@
 # Insecure Deserialization
 
-## Insecure Deserialization in XYZ
-
-There is a SQL Injection in `User Search` feature at the following URL  
+There input file to legacy bulk import feature does not securely deserialize the causing  
 
 http://127.0.0.1:9090/app/usersearch