From aaf549d7870b8687209a3282841b59207735b676 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 0000000000000..1a6b6133d66b9
--- /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 234c774c3a6cb..b49e808845d73 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 0000000000000..8845b486294c3
--- /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 2dab84be563b0..a9b8f1ca22998 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'