CVE-2019-10767: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: An attacker can include file contents from outside the `/adapter/xxx/` directory, where `xxx` is the name of an existent adapter like "admin". It is exploited using the administrative web panel with a request for an adapter file. **Note:** The attacker has to be logged in if the authentication is enabled (by default isn't enabled).
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 7.5
Published: 2019-11-21T17:15Z
Repository: https://github.com/ioBroker/ioBroker.js-controller
Language: JavaScript
Files changed: lib/objects/objectsUtils.js, test/lib/testObjects.js
Commit date: 2019-10-04 20:24:49 +0200
Commit message: some security enhancements to better prevent path traversing

Diff: diff --git a/lib/objects/objectsUtils.js b/lib/objects/objectsUtils.js
index e23c481..d6df772 100644
--- a/lib/objects/objectsUtils.js
+++ b/lib/objects/objectsUtils.js
@@ -470,15 +470,21 @@ function sanitizePath(id, name, callback) {
         }
         return;
     }
 
     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
 
     return {id: id, name: name};
diff --git a/test/lib/testObjects.js b/test/lib/testObjects.js
index add4c60..06d49b2 100644
--- a/test/lib/testObjects.js
+++ b/test/lib/testObjects.js
@@ -339,10 +339,43 @@ 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 => {
             expect(err).to.be.not.ok;
             objects.unlink(testId, 'myFile/abc1.txt', err => {

Vulnerable code:         id = id.replace(/\.\./g, ''); // do not allow to write in parent directories
    if (name.indexOf('..') !== -1) {
        name = path.normalize(name);
Fixed code:         id = id.replace(/[\]\[*,;'"`<>\\?\/]/g, ''); // remove all invalid characters from states plus slashes
    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);
    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();
                                });
                            });
                        });
                    });
                });
            });
        });
    });