Improve error handling and testing of XML attacks

shawnsnyk · Jan 25, 2018 · 9f85a67 · 9f85a67
1 parent 30623bb
commit 9f85a67
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 19 deletions.
diff --git a/ftp/suspicious_errors.yml b/ftp/suspicious_errors.yml
@@ -14,5 +14,6 @@ detection:
         - 'Unrecognized target URL for redirect: *'
         - 'B2B customer complaints via file upload have been deprecated for security reasons'
         - 'Infinite loop detected'
+        - 'Detected an entity reference loop'
     condition: keywords
 level: low
diff --git a/routes/b2bOrder.js b/routes/b2bOrder.js
@@ -13,12 +13,15 @@ exports = module.exports = function b2bOrder () {
         vm.createContext(sandbox)
         vm.runInContext('safeEval(orderLineData)', sandbox, { timeout: 2000 })
       } catch (err) {
-        if (utils.notSolved(challenges.rceChallenge) && err.message === 'Infinite loop detected - reached max iterations') {
-          utils.solve(challenges.rceChallenge)
+        if (err.message === 'Infinite loop detected - reached max iterations') {
+          if (utils.notSolved(challenges.rceChallenge)) {
+            utils.solve(challenges.rceChallenge)
+          }
           next(err)
-        }
-        if (utils.notSolved(challenges.rceOccupyChallenge) && err.message === 'Script execution timed out.') {
-          utils.solve(challenges.rceOccupyChallenge)
+        } else if (err.message === 'Script execution timed out.') {
+          if (utils.notSolved(challenges.rceOccupyChallenge)) {
+            utils.solve(challenges.rceOccupyChallenge)
+          }
         }
       }
     })

diff --git a/routes/fileUpload.js b/routes/fileUpload.js
@@ -19,9 +19,7 @@ exports = module.exports = function fileUpload () {
       }
       if (file.buffer) {
         const data = file.buffer.toString()
-        if (utils.contains(data, '/dev/random')) { // circuit breaker to prevent common DoS attack
-          next(new Error('Blocked illegal activity by ' + req.connection.remoteAddress))
-        }
+        res.status(410)
         try {
           const sandbox = { libxml, data }
           vm.createContext(sandbox)
@@ -30,14 +28,9 @@ exports = module.exports = function fileUpload () {
           if (utils.notSolved(challenges.xxeFileDisclosureChallenge) && (matchesSystemIniFile(xmlString) || matchesEtcPasswdFile(xmlString))) {
             utils.solve(challenges.xxeFileDisclosureChallenge)
           }
-          res.status(410)
-          next(new Error('B2B customer complaints via file upload have been deprecated for security reasons!\n' + xmlString))
+          next(new Error('B2B customer complaints via file upload have been deprecated for security reasons: ' + xmlString))
         } catch (err) {
-          if (utils.notSolved(challenges.rceOccupyChallenge) && err.message === 'Script execution timed out.') {
-            utils.solve(challenges.rceOccupyChallenge)
-          }
-          res.status(410)
-          next(new Error('B2B customer complaints via file upload have been deprecated for security reasons!\n' + err))
+          next(new Error('B2B customer complaints via file upload have been deprecated for security reasons: ' + err.message))
         }
       }
     }

diff --git a/test/api/b2bOrderSpec.js b/test/api/b2bOrderSpec.js
@@ -15,8 +15,6 @@ describe('/b2b/v2/orders', () => {
       }
     })
       .expect('status', 500)
-      .expect('header', 'content-type', /text\/html/)
-      .expect('bodyContains', '<h1>Juice Shop (Express ~')
       .expect('bodyContains', 'Infinite loop detected - reached max iterations')
       .done(done)
   })

diff --git a/test/api/fileUploadSpec.js b/test/api/fileUploadSpec.js
@@ -72,13 +72,24 @@ describe('/file-upload', () => {
     })
   }
 
-  it('POST file type XML with /dev/random DoS attack is blocked', done => {
+  it('POST file type XML with /dev/random DoS attack responds after timeout', done => {
     file = path.resolve(__dirname, '../files/xxeDoS.xml')
     form = new FormData()
     form.append('file', fs.createReadStream(file))
 
     frisby.post(URL + '/file-upload', { headers: form.getHeaders(), body: form })
-      .expect('status', 500)
+      .expect('status', 410)
+      .done(done)
+  })
+
+  it('POST file type XML with Billion Laughs attack responds', done => {
+    file = path.resolve(__dirname, '../files/xmlBillionLaughs.xml')
+    form = new FormData()
+    form.append('file', fs.createReadStream(file))
+
+    frisby.post(URL + '/file-upload', { headers: form.getHeaders(), body: form })
+      .expect('status', 410)
+      .expect('bodyContains', 'Detected an entity reference loop')
       .done(done)
   })
 

diff --git a/test/files/xmlBillionLaughs.xml b/test/files/xmlBillionLaughs.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0"?>
+<!DOCTYPE lolz [
+        <!ENTITY lol "lol">
+        <!ELEMENT lolz (#PCDATA)>
+        <!ENTITY lol1 "&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;">
+        <!ENTITY lol2 "&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;">
+        <!ENTITY lol3 "&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;">
+        <!ENTITY lol4 "&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;">
+        <!ENTITY lol5 "&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;">
+        <!ENTITY lol6 "&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;">
+        <!ENTITY lol7 "&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;">
+        <!ENTITY lol8 "&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;">
+        <!ENTITY lol9 "&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;">
+        ]>
+<lolz>&lol9;</lolz>