some security enhancements to better prevent path traversing

ioBroker · Oct 4, 2019 · f6e292c · f6e292c
1 parent fbdbfbd
commit f6e292c
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 3 deletions.
diff --git a/lib/objects/objectsUtils.js b/lib/objects/objectsUtils.js
@@ -472,11 +472,17 @@ function sanitizePath(id, name, callback) {
     }
 
     if (id) {
-        id = id.replace(/\.\./g, ''); // do not allow to write in parent directories
+        id = id.replace(/[\]\[*,;'"`<>\\?\/]/g, ''); // remove all invalid characters from states plus slashes
     }
 
-    if (name.indexOf('..') !== -1) {
-        name = path.normalize(name);
+    if (name.includes('..')) {
+        name = path.normalize('/' + name);
+        name = name.replace(/\\/g, '/');
+    }
+    if (name.includes('..')) {
+        // Also after normalization we still have .. in it - should not happen if normalize worked correctly
+        name = name.replace(/\.\./g, '');
+        name = path.normalize('/' + name);
         name = name.replace(/\\/g, '/');
     }
     if (name[0] === '/') name = name.substring(1); // do not allow absolute paths

diff --git a/test/lib/testObjects.js b/test/lib/testObjects.js
@@ -341,6 +341,39 @@ function register(it, expect, context) {
         });
     });
 
+    it(textName + 'should read file and prevent path traversing', done => {
+        const objects = context.objects;
+        objects.readFile(testId, '../../myFile/abc1.txt', err => {
+            expect(err).to.be.not.ok;
+            expect(data).to.be.equal('dataInFile');
+            objects.readFile(testId, '/myFile/abc1.txt', err => {
+                expect(err).to.be.not.ok;
+                expect(data).to.be.equal('dataInFile');
+                objects.readFile(testId, '/../../myFile/abc1.txt', err => {
+                    expect(err).to.be.not.ok;
+                    expect(data).to.be.equal('dataInFile');
+                    objects.readFile(testId, 'myFile/../blubb/../myFile/abc1.txt', err => {
+                        expect(err).to.be.not.ok;
+                        expect(data).to.be.equal('dataInFile');
+                        objects.readFile(testId, '/myFile/../blubb/../myFile/abc1.txt', err => {
+                            expect(err).to.be.not.ok;
+                            expect(data).to.be.equal('dataInFile');
+                            objects.readFile(testId, '../blubb/../myFile/abc1.txt', err => {
+                                expect(err).to.be.not.ok;
+                                expect(data).to.be.equal('dataInFile');
+                                objects.readFile(testId, '/../blubb/../myFile/abc1.txt', err => {
+                                    expect(err).to.be.not.ok;
+                                    expect(data).to.be.equal('dataInFile');
+                                    done();
+                                });
+                            });
+                        });
+                    });
+                });
+            });
+        });
+    });
+
     it(textName + 'should unlink file', done => {
         const objects = context.objects;
         objects.unlink(testId, 'myFile/abc1.txt', err => {