search

apache / cordova-android

Apache Cordova Android
https://cordova.apache.org/
Apache License 2.0
3.66k stars 1.54k forks source link

Prototype pollution in dot-prop #935

Closed ThonyBG closed 4 years ago

ThonyBG commented 4 years ago

Bug Report

I would like to report a parameter pollution in dot-prop It allows an attacker to modify the prototype of a base object which can vary in severity depending on the implementation (DoS, access to sensitive data, RCE).

Problem

cordova-android 8.1.0 has a dependency of compare-func that has an outdated dependency of dot-prop 3.0.0, this version of dot-prop was reported with a security issue "Prototype pollution"

What is expected to happen?

Do not use libraries with security issues

What does actually happen?

cordova-android is firing security issue in all scans such as Black Duck, snyk

Information

Related issue reports:

https://hackerone.com/reports/719856 https://www.openhub.net/p/737898 https://snyk.io/test/npm/dot-prop/3.0.0

Command or Code

Environment, Platform, Device

Version information

cordova-android 8-1-0

Checklist

timbru31 commented 4 years ago

Thanks for the report. I'd vote to replace the outdated, unmaintained compare-func dependency and rewrite the one and only usage:

https://github.com/apache/cordova-android/blob/de105e8651bcd8227bc2acdb91a21a14d2dc59c7/bin/templates/cordova/lib/builders/ProjectBuilder.js#L36-L48

breautek commented 4 years ago

Installing cordova-android does not yield any security vulnerabilities for me (assuming they are reported to NPM?). In my experience this is caused by having a sub-dependency installed at some point, and they don't get updated, even if you reinstall cordova-android.

I'd try doing the following in your app:

rm package-lock.json
rm -r node_modules
npm install

This should cause all your dependencies to install using the latest satisfactory versions across your dependency tree.

timbru31 commented 4 years ago

Snyk does report it though: https://snyk.io/test/npm/cordova-android/8.1.0

breautek commented 4 years ago

Yes, https://github.com/stevemao/compare-func/blob/master/package.json#L33 compare-func is using a flexible version, so as long as dot-prop has a fix inside 3.x.x version, clearing out the node modules and reinstalling should address the security vulnerability.

But I agree, we can probably just remove the compare-func dependency, doing this with the native sort method isn't all that difficult...

ThonyBG commented 4 years ago

Yes, https://github.com/stevemao/compare-func/blob/master/package.json#L33 compare-func is using a flexible version, so as long as dot-prop has a fix inside 3.x.x version, clearing out the node modules and reinstalling should address the security vulnerability.

But I agree, we can probably just remove the compare-func dependency, doing this with the native sort method isn't all that difficult...

Remove node_modules is not a possibility to who has a security scan running as step into theirs ci/cd pipeline. Even when circleci is running in a docker, and it's installing all dependecies from scrach all security scanners detec this vulnerability

v-ko commented 3 years ago

Hi, sorry for excavating the issue, but I recently started working on a cordova deployment for an app and got the warning from GitHub for the dot-prop package being a security vulnerability (< 4.2.1). I created the package with cordova create, following the tutorial on the cordova website. Should I raise that as a new issue? Cordova CLI reports version 9.0.0

breautek commented 3 years ago

Hi, sorry for excavating the issue, but I recently started working on a cordova deployment for an app and got the warning from GitHub for the dot-prop package being a security vulnerability (< 4.2.1). I created the package with cordova create, following the tutorial on the cordova website. Should I raise that as a new issue? Cordova CLI reports version 9.0.0

Cordova-android doesn't use the package that uses dot-prop anymore:

Cordova-android@8.1.0

io.cordova.hellocordova@1.0.0 /development/cordova/tests/dotproptest
└─┬ cordova-android@8.1.0
  └─┬ compare-func@1.3.4
    └── dot-prop@3.0.0

Cordova-android@9

npm ls dot-prop
io.cordova.hellocordova@1.0.0 /development/cordova/tests/dotproptest
└── (empty)

Here is the commit that removes compare-func (which has the dot-prop dependency).

https://github.com/apache/cordova-android/commit/8ab1dbc373eefd55e912e29d6bfb2a9c67e95b6e#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519

v-ko commented 3 years ago

Ok, thanks for the response. I guess I had the cordova-android package cached and the version that's installed by cordova-create is 8.1.0, because that's what npm ls dot-prop reports.