Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

@googlebot I signed it!

Great job! I have created a fork in the meantime :) (some owner please merge it)

This fix sometimes fails, presumably when outputDirectory is not itself canonical.
Had to revert it in our project.

can somebody merge this?????

This fix sometimes fails, presumably when outputDirectory is not itself canonical. Had to revert it in our project.

When the outputDirectory is a relative path you can convert it to a canonical path and still apply the same check.

  File file = new File(outputDirectory + compressedName);
  String canonicalDestinationPath = (new File(outputDirectory)).getCanonicalPath();
  String canonicalPath = file.getCanonicalPath();
  if (!canonicalPath.startsWith(canonicalDestinationPath)) {
      String errorMessage = "Zip traversal security error";
      callbackContext.error(errorMessage);
      Log.e(LOG_TAG, errorMessage);
      return;
  }

Should this really be approved, if it fails when the outputDirectory is not canonical? The code snippet by @DavidTalevski should be the way to go, or is there some weird behavior that occurs with it?

@lczw I've tested the solution for both relative and full paths and it works for both. So far I have not encountered any weird behaviors.

I see. I have encountered a problem where there was a mismatch which is fixed by getting the canonical path of the output directory.

However, if it works on your end then it's probably something that should be fixed in my project instead of the plugin ^^

I see. I have encountered a problem where there was a mismatch which is fixed by getting the canonical path of the output directory.

However, if it works on your end then it's probably something that should be fixed in my project instead of the plugin ^^

I was describing the getCanonicalPath solution. I also had an issue with the PR code with relative paths which got fixed when turning them to canonical, so the problem is most likely not on your end.

I misunderstood your comment, sorry. Your code works fine for me as well, which is why I though it's a good idea to use it instead of the PR's code

So is the final decision that this will not be merged?

Hi devs,

I've manually applied this fix into my code today and would like to share a note about the problem. The fix was applied to an Android APP (Ionic v3 / Angular) that uses the cordova-plugin-zip lib. After apply the fix and test the APP, I noticed that the zip content that I had into my APP was not being unzipped, and while checking logs with adb logcat, I saw the message "Zip traversal security error" from the fix applied.

After debug the problem, I saw that, in my APP, the output directory where the content was being extracted is the user directory inside the Android filesystem. Compare the directories below:

Output from "file.getCanonicalPath()":
/data/data/br.com.someapplication.sub/files/...

My APP output directory (where the zip content would be extract to):
/data/user/0/br.com.someapplication.sub/files/...

Because of this, the code if (!canonicalPath.startsWith(outputDirectory)... fails and my zip files are not unzipped.

So, to be able to use the fix #92 and attend my scenario, I've added one more verification, as below:

...
File file = new File(outputDirectory + compressedName);
String canonicalPath = file.getCanonicalPath();
String absolutePath = file.getAbsolutePath();
if (!canonicalPath.startsWith(outputDirectory) && !absolutePath.startsWith(outputDirectory)) {
    String errorMessage = "Zip traversal security error";
    ...

Hope this can help someone.

@jcperuffo we ran into the same problem & your solution worked. thanks!
I made a fork: https://github.com/bikubi/cordova-plugin-zip/ or see commit above.

This fix sometimes fails, presumably when outputDirectory is not itself canonical. Had to revert it in our project.

When the outputDirectory is a relative path you can convert it to a canonical path and still apply the same check.

  File file = new File(outputDirectory + compressedName);
  String canonicalDestinationPath = (new File(outputDirectory)).getCanonicalPath();
  String canonicalPath = file.getCanonicalPath();
  if (!canonicalPath.startsWith(canonicalDestinationPath)) {
      String errorMessage = "Zip traversal security error";
      callbackContext.error(errorMessage);
      Log.e(LOG_TAG, errorMessage);
      return;
  }

I think this is the fix we want.