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'