Merge pull request #319 from hindessm/restrict-library-entry-names

Prohibit library entry names from containing '../'.

red/library.js

@@ -34,6 +34,10 @@ function init() {
                     res.send(204);
                 }).otherwise(function(err) {
                     util.log("[red] Error loading flow '"+req.params[0]+"' : "+err);
+                    if (err.message.indexOf('forbidden') === 0) {
+                        res.send(403);
+                        return;
+                    }
                     res.send(500);
                 });
             });
@@ -52,6 +56,10 @@ function init() {
             }).otherwise(function(err) {
                 if (err) {
                     util.log("[red] Error loading flow '"+req.params[0]+"' : "+err);
+                    if (err.message.indexOf('forbidden') === 0) {
+                        res.send(403);
+                        return;
+                    }
                 }
                 res.send(404);
             });
@@ -75,6 +83,10 @@ function createLibrary(type) {
             }).otherwise(function(err) {
                 if (err) {
                     util.log("[red] Error loading library entry '"+path+"' : "+err);
+                    if (err.message.indexOf('forbidden') === 0) {
+                        res.send(403);
+                        return;
+                    }
                 }
                 res.send(404);
             });
@@ -91,6 +103,10 @@ function createLibrary(type) {
                         res.send(204);
                     }).otherwise(function(err) {
                         util.log("[red] Error saving library entry '"+path+"' : "+err);
+                        if (err.message.indexOf('forbidden') === 0) {
+                            res.send(403);
+                            return;
+                        }
                         res.send(500);
                     });
             });

red/storage/index.js

@@ -33,6 +33,10 @@ function moduleSelector(aSettings) {
     return toReturn;
 }
 
+function is_malicious(path) {
+    return path.indexOf('../') != -1 || path.indexOf('..\\') != -1;
+}
+
 var storageModuleInterface = {
         init : function(settings) {
             try {
@@ -58,15 +62,27 @@ var storageModuleInterface = {
             return storageModule.getAllFlows();
         },
         getFlow : function(fn) {
+            if (is_malicious(fn)) {
+                return when.reject(new Error('forbidden flow name'));
+            }
             return storageModule.getFlow(fn);
         },
         saveFlow : function(fn, data) {
+            if (is_malicious(fn)) {
+                return when.reject(new Error('forbidden flow name'));
+            }
             return storageModule.saveFlow(fn, data);
         },
         getLibraryEntry : function(type, path) {
+            if (is_malicious(path)) {
+                return when.reject(new Error('forbidden flow name'));
+            }
             return storageModule.getLibraryEntry(type, path);
         },
         saveLibraryEntry : function(type, path, meta, body) {
+            if (is_malicious(path)) {
+                return when.reject(new Error('forbidden flow name'));
+            }
             return storageModule.saveLibraryEntry(type, path, meta, body);
         }
 }

test/red/library_spec.js

@@ -120,6 +120,27 @@ describe("library", function() {
                         });
                 });
         });
+
+        it('returns 403 for malicious access attempt', function(done) {
+            // without the userDir override the malicious url would be
+            // http://127.0.0.1:1880/library/flows/../../package to
+            // obtain package.json from the node-red root.
+            request(RED.httpAdmin)
+                .get('/library/flows/../../../../../package')
+                .expect(403)
+                .end(done);
+        });
+
+        it('returns 403 for malicious access attempt', function(done) {
+            // without the userDir override the malicious url would be
+            // http://127.0.0.1:1880/library/flows/../../package to
+            // obtain package.json from the node-red root.
+            request(RED.httpAdmin)
+                .post('/library/flows/../../../../../package')
+                .expect(403)
+                .end(done);
+        });
+
     });
 
     describe("type", function() {
@@ -188,5 +209,29 @@ describe("library", function() {
                 });
         });
 
+
+        it('returns 403 for malicious access attempt', function(done) {
+            request(RED.httpAdmin)
+                .get('/library/test/../../../../../../../../../../etc/passwd')
+                .expect(403)
+                .end(done);
+        });
+
+        it('returns 403 for malicious access attempt', function(done) {
+            request(RED.httpAdmin)
+                .get('/library/test/..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\etc\\passwd')
+                .expect(403)
+                .end(done);
+        });
+
+        it('returns 403 for malicious access attempt', function(done) {
+            request(RED.httpAdmin)
+                .post('/library/test/../../../../../../../../../../etc/passwd')
+                .set('Content-Type', 'text/plain')
+                .send('root:x:0:0:root:/root:/usr/bin/tclsh')
+                .expect(403)
+                .end(done);
+        });
+
     });
 });

test/red/storage/index_spec.js

@@ -83,19 +83,19 @@ describe("red/storage/index", function() {
                     calledFlagGetAllFlows = true;
                 },
                 getFlow : function(fn) {
-                    fn.should.be.true;
+                    fn.should.equal("name");
                 },
                 saveFlow : function(fn, data) {
-                    fn.should.be.true;
+                    fn.should.equal("name");
                     data.should.be.true;
                 },
                 getLibraryEntry : function(type, path) {
                     type.should.be.true;
-                    path.should.be.true;
+                    path.should.equal("name");
                 },
                 saveLibraryEntry : function(type, path, meta, body) {
                     type.should.be.true;
-                    path.should.be.true;
+                    path.should.equal("name");
                     meta.should.be.true;
                     body.should.be.true;
                 }
@@ -112,10 +112,10 @@ describe("red/storage/index", function() {
         storage.getCredentials(); 
         storage.saveCredentials(true);
         storage.getAllFlows();
-        storage.getFlow(true);
+        storage.getFlow("name");
-        storage.saveFlow(true, true);
+        storage.saveFlow("name", true);
-        storage.getLibraryEntry(true, true);
+        storage.getLibraryEntry(true, "name");
-        storage.saveLibraryEntry(true, true, true, true);
+        storage.saveLibraryEntry(true, "name", true, true);
         
         calledInit.should.be.true;
         calledFlagGetFlows.should.be.true;