From 1f304ef372b1c247f357ae5af3499bf35f2e1a57 Mon Sep 17 00:00:00 2001
From: Sam Doran <sdoran@redhat.com>
Date: Fri, 28 Feb 2020 17:56:21 -0500
Subject: [PATCH] win_unzip  - normalize and compare paths to prevent path
 traversal (#67799)

* Actually inspect the paths and prevent escape
* Add integration tests
* Generate zip files for use in integration test
* Adjust error message

(cherry picked from commit d30c57ab22db24f6901166fcc3155667bdd3443f)
---
 .../win-unzip-check-extraction-path.yml       |  4 ++
 lib/ansible/modules/windows/win_unzip.ps1     |  9 +++
 .../files/create_crafty_zip_files.py          | 65 +++++++++++++++++++
 .../targets/win_unzip/tasks/main.yml          | 57 +++++++++++++++-
 4 files changed, 134 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/fragments/win-unzip-check-extraction-path.yml
 create mode 100644 test/integration/targets/win_unzip/files/create_crafty_zip_files.py

diff --git a/changelogs/fragments/win-unzip-check-extraction-path.yml b/changelogs/fragments/win-unzip-check-extraction-path.yml
new file mode 100644
index 00000000000..1a6b6133d66
--- /dev/null
+++ b/changelogs/fragments/win-unzip-check-extraction-path.yml
@@ -0,0 +1,4 @@
+bugfixes:
+  - >
+    **security issue** win_unzip - normalize paths in archive to ensure extracted
+    files do not escape from the target directory (CVE-2020-1737)
diff --git a/lib/ansible/modules/windows/win_unzip.ps1 b/lib/ansible/modules/windows/win_unzip.ps1
index 234c774c3a6..b49e808845d 100644
--- a/lib/ansible/modules/windows/win_unzip.ps1
+++ b/lib/ansible/modules/windows/win_unzip.ps1
@@ -40,6 +40,15 @@ Function Extract-Zip($src, $dest) {
         $entry_target_path = [System.IO.Path]::Combine($dest, $archive_name)
         $entry_dir = [System.IO.Path]::GetDirectoryName($entry_target_path)
 
+        # Normalize paths for further evaluation
+        $full_target_path = [System.IO.Path]::GetFullPath($entry_target_path)
+        $full_dest_path = [System.IO.Path]::GetFullPath($dest + [System.IO.Path]::DirectorySeparatorChar)
+
+        # Ensure file in the archive does not escape the extraction path
+        if (-not $full_target_path.StartsWith($full_dest_path)) {
+            Fail-Json -obj $result -message "Error unzipping '$src' to '$dest'! Filename contains relative paths which would extract outside the destination: $entry_target_path"
+        }
+
         if (-not (Test-Path -LiteralPath $entry_dir)) {
             New-Item -Path $entry_dir -ItemType Directory -WhatIf:$check_mode | Out-Null
             $result.changed = $true
diff --git a/test/integration/targets/win_unzip/files/create_crafty_zip_files.py b/test/integration/targets/win_unzip/files/create_crafty_zip_files.py
new file mode 100644
index 00000000000..8845b486294
--- /dev/null
+++ b/test/integration/targets/win_unzip/files/create_crafty_zip_files.py
@@ -0,0 +1,65 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+# Copyright (c) 2020 Ansible Project
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+import os
+import shutil
+import sys
+import zipfile
+
+# Each key is a zip file and the vaule is the list of files that will be created
+# and placed in the archive
+zip_files = {
+    'hat1': [r'hat/..\rabbit.txt'],
+    'hat2': [r'hat/..\..\rabbit.txt'],
+    'handcuffs': [r'..\..\houidini.txt'],
+    'prison': [r'..\houidini.txt'],
+}
+
+# Accept an argument of where to create the files, defaulting to
+# the current working directory.
+try:
+    output_dir = sys.argv[1]
+except IndexError:
+    output_dir = os.getcwd()
+
+if not os.path.isdir(output_dir):
+    os.mkdir(output_dir)
+
+os.chdir(output_dir)
+
+for name, files in zip_files.items():
+    # Create the files to go in the zip archive
+    for entry in files:
+        dirname = os.path.dirname(entry)
+        if dirname:
+            if os.path.isdir(dirname):
+                shutil.rmtree(dirname)
+            os.mkdir(dirname)
+
+        with open(entry, 'w') as e:
+            e.write('escape!\n')
+
+    # Create the zip archive with the files
+    filename = '%s.zip' % name
+    if os.path.isfile(filename):
+        os.unlink(filename)
+
+    with zipfile.ZipFile(filename, 'w') as zf:
+        for entry in files:
+            zf.write(entry)
+
+    # Cleanup
+    if dirname:
+        shutil.rmtree(dirname)
+
+    for entry in files:
+        try:
+            os.unlink(entry)
+        except OSError:
+            pass
diff --git a/test/integration/targets/win_unzip/tasks/main.yml b/test/integration/targets/win_unzip/tasks/main.yml
index 2dab84be563..a9b8f1ca229 100644
--- a/test/integration/targets/win_unzip/tasks/main.yml
+++ b/test/integration/targets/win_unzip/tasks/main.yml
@@ -1,4 +1,3 @@
----
 - name: create test directory
   win_file:
     path: '{{ win_unzip_dir }}\output'
@@ -114,3 +113,59 @@
     - unzip_delete is changed
     - unzip_delete.removed
     - not unzip_delete_actual.stat.exists
+
+# Path traversal tests (CVE-2020-1737)
+- name: Create zip files
+  script: create_crafty_zip_files.py {{ output_dir }}
+  delegate_to: localhost
+
+- name: Copy zip files to Windows host
+  win_copy:
+    src: "{{ output_dir }}/{{ item }}.zip"
+    dest: "{{ win_unzip_dir }}/"
+  loop:
+    - hat1
+    - hat2
+    - handcuffs
+    - prison
+
+- name: Perform first trick
+  win_unzip:
+    src: '{{ win_unzip_dir }}\hat1.zip'
+    dest: '{{ win_unzip_dir }}\output'
+  register: hat_trick1
+
+- name: Check for file
+  win_stat:
+    path: '{{ win_unzip_dir }}\output\rabbit.txt'
+  register: rabbit
+
+- name: Perform next tricks (which should all fail)
+  win_unzip:
+    src: '{{ win_unzip_dir }}\{{ item }}.zip'
+    dest: '{{ win_unzip_dir }}\output'
+  ignore_errors: yes
+  register: escape
+  loop:
+    - hat2
+    - handcuffs
+    - prison
+
+- name: Search for files
+  win_find:
+    recurse: yes
+    paths:
+      - '{{ win_unzip_dir }}'
+    patterns:
+      - '*houdini.txt'
+      - '*rabbit.txt'
+  register: files
+
+- name: Check results
+  assert:
+    that:
+      - rabbit.stat.exists
+      - hat_trick1 is success
+      - escape.results | map(attribute='failed') | unique | list == [True]
+      - files.matched == 1
+      - files.files[0]['filename'] == 'rabbit.txt'