You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by sh...@apache.org on 2015/12/21 23:44:02 UTC

cordova-lib git commit: CB-7183 prevent read/write/modify files outside project from plugins

Repository: cordova-lib
Updated Branches:
  refs/heads/master d5842d651 -> 5eb351e08


CB-7183 prevent read/write/modify files outside project from plugins

This closes #355


Project: http://git-wip-us.apache.org/repos/asf/cordova-lib/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-lib/commit/5eb351e0
Tree: http://git-wip-us.apache.org/repos/asf/cordova-lib/tree/5eb351e0
Diff: http://git-wip-us.apache.org/repos/asf/cordova-lib/diff/5eb351e0

Branch: refs/heads/master
Commit: 5eb351e08eb9b2c7c685b7c06a4fc50316bcce02
Parents: d5842d6
Author: Byoungro So <by...@intel.com>
Authored: Wed Dec 9 19:43:46 2015 -0800
Committer: Shazron Abdullah <sh...@apache.org>
Committed: Mon Dec 21 14:42:21 2015 -0800

----------------------------------------------------------------------
 cordova-lib/spec-plugman/install.spec.js        | 17 ++++++++++++
 .../org.test.invalid.engine.script/plugin.xml   | 28 ++++++++++++++++++++
 cordova-lib/src/plugman/install.js              | 13 +++++----
 3 files changed, 53 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/5eb351e0/cordova-lib/spec-plugman/install.spec.js
----------------------------------------------------------------------
diff --git a/cordova-lib/spec-plugman/install.spec.js b/cordova-lib/spec-plugman/install.spec.js
index 50a81ad..8420511 100644
--- a/cordova-lib/spec-plugman/install.spec.js
+++ b/cordova-lib/spec-plugman/install.spec.js
@@ -49,6 +49,7 @@ var install = require('../src/plugman/install'),
         'org.test.plugins.childbrowser' : path.join(plugins_dir, 'org.test.plugins.childbrowser'),
         'com.adobe.vars' : path.join(plugins_dir, 'com.adobe.vars'),
         'org.test.defaultvariables' : path.join(plugins_dir, 'org.test.defaultvariables'),
+        'org.test.invalid.engine.script' : path.join(plugins_dir, 'org.test.invalid.engine.script'),
         'A' : path.join(plugins_dir, 'dependencies', 'A'),
         'B' : path.join(plugins_dir, 'dependencies', 'B'),
         'C' : path.join(plugins_dir, 'dependencies', 'C'),
@@ -503,6 +504,22 @@ describe('install', function() {
                 expect(''+done).toMatch(true);
             });
         });
+        it('should throw if the engine scriptSrc escapes out of the plugin dir.', function(done) {
+            var success = jasmine.createSpy('success');
+            spyOn(PlatformJson.prototype, 'isPluginInstalled').andReturn(false);
+            install('android', project, plugins['org.test.invalid.engine.script'])
+                .then(success)
+                .fail(function(err) {
+                    // <engine name="path-escaping-plugin" version=">=1.0.0" scriptSrc="../../../malicious/script" platform="*" />
+                    expect(err).toBeDefined();
+                    expect(err.message.indexOf('security violation:')).toBe(0);
+                    done();
+                })
+                .fin(function() {
+                    expect(success).not.toHaveBeenCalled();
+                    done();
+                });
+        });
     });
 });
 

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/5eb351e0/cordova-lib/spec-plugman/plugins/org.test.invalid.engine.script/plugin.xml
----------------------------------------------------------------------
diff --git a/cordova-lib/spec-plugman/plugins/org.test.invalid.engine.script/plugin.xml b/cordova-lib/spec-plugman/plugins/org.test.invalid.engine.script/plugin.xml
new file mode 100644
index 0000000..aba667e
--- /dev/null
+++ b/cordova-lib/spec-plugman/plugins/org.test.invalid.engine.script/plugin.xml
@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<plugin xmlns="http://www.phonegap.com/ns/plugins/1.0"
+    id="org.test.invalid.engine.script"
+    version="1.0.0">
+
+    <name>Engine Choo Choo</name>
+
+    <engines>
+        <engine name="scriptSrc-escaping-plugin" version=">=1.0.0" scriptSrc="../../../malicious/script" platform="*" />
+    </engines>
+</plugin>

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/5eb351e0/cordova-lib/src/plugman/install.js
----------------------------------------------------------------------
diff --git a/cordova-lib/src/plugman/install.js b/cordova-lib/src/plugman/install.js
index ec1fc76..4632623 100644
--- a/cordova-lib/src/plugman/install.js
+++ b/cordova-lib/src/plugman/install.js
@@ -182,14 +182,12 @@ function cleanVersionOutput(version, name){
 
 // exec engine scripts in order to get the current engine version
 // Returns a promise for the array of engines.
-function callEngineScripts(engines) {
+function callEngineScripts(engines, project_dir) {
 
     return Q.all(
         engines.map(function(engine){
             // CB-5192; on Windows scriptSrc doesn't have file extension so we shouldn't check whether the script exists
-
             var scriptPath = engine.scriptSrc ? '"' + engine.scriptSrc + '"' : null;
-
             if(scriptPath && (isWindows || fs.existsSync(engine.scriptSrc)) ) {
 
                 var d = Q.defer();
@@ -256,8 +254,13 @@ function getEngines(pluginInfo, platform, project_dir, plugin_dir){
         // check for other engines
         }else{
             platformIndex = engine.platform.indexOf(platform);
+            // CB-7183: security check for scriptSrc path escaping outside the plugin
+            var scriptSrcPath = path.resolve(plugin_dir, engine.scriptSrc);
+            if (scriptSrcPath.indexOf(plugin_dir) !== 0) {
+                throw new Error('security violation: scriptSrc '+scriptSrcPath+' is out of plugin dir '+plugin_dir);
+            }
             if(platformIndex > -1 || engine.platform === '*'){
-                uncheckedEngines.push({ 'name': theName, 'platform': engine.platform, 'scriptSrc':path.resolve(plugin_dir, engine.scriptSrc), 'minVersion' :  engine.version});
+                uncheckedEngines.push({ 'name': theName, 'platform': engine.platform, 'scriptSrc':scriptSrcPath, 'minVersion' :  engine.version});
             }
         }
     });
@@ -319,7 +322,7 @@ function runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, opt
         return Q(superspawn.maybeSpawn(path.join(project_dir, 'cordova', 'version'), [], { chmod: true }));
     }).then(function(platformVersion) {
         options.platformVersion = platformVersion;
-        return callEngineScripts(theEngines);
+        return callEngineScripts(theEngines, path.resolve(plugins_dir, '..'));
     }).then(function(engines) {
         return checkEngines(engines);
     }).then(function() {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org