From 8cccb33d71b0822c46f7076aa28293968eb25438 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Tue, 14 Apr 2020 20:44:45 -0400 Subject: [stable-2.7] win_unzip - normalize and compare paths to prevent path traversal (#67799) (#67938) * 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) * Fix tests for 2.7 --- .../fragments/win-unzip-check-extraction-path.yml | 4 ++ lib/ansible/modules/windows/win_unzip.ps1 | 9 +++ .../targets/win_unzip/defaults/main.yml | 1 + .../win_unzip/files/create_crafty_zip_files.py | 65 +++++++++++++++++++++ test/integration/targets/win_unzip/tasks/main.yml | 66 ++++++++++++++++++++++ 5 files changed, 145 insertions(+) create mode 100644 changelogs/fragments/win-unzip-check-extraction-path.yml create mode 100644 test/integration/targets/win_unzip/defaults/main.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 0000000000..1a6b6133d6 --- /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 4bcf9a9406..9051002ce0 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 -Path $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/defaults/main.yml b/test/integration/targets/win_unzip/defaults/main.yml new file mode 100644 index 0000000000..3ba79a5558 --- /dev/null +++ b/test/integration/targets/win_unzip/defaults/main.yml @@ -0,0 +1 @@ +win_unzip_dir: 'C:\Windows\Temp\win_unzip_test' 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 0000000000..8845b48629 --- /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 ed4dc22877..325b396196 100644 --- a/test/integration/targets/win_unzip/tasks/main.yml +++ b/test/integration/targets/win_unzip/tasks/main.yml @@ -14,3 +14,69 @@ vars: in_check_mode: yes check_mode: yes + +- name: cleanup test directory + win_file: + path: '{{ win_unzip_dir }}\output' + state: absent + +- name: create test directory + win_file: + path: '{{ win_unzip_dir }}\output' + state: directory + +# 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' -- cgit v1.2.1