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
This commit is contained in:
Sam Doran 2020-02-28 17:56:21 -05:00 committed by GitHub
parent 8ab304af44
commit d30c57ab22
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 134 additions and 1 deletions

View file

@ -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)

View file

@ -40,6 +40,15 @@ Function Extract-Zip($src, $dest) {
$entry_target_path = [System.IO.Path]::Combine($dest, $archive_name) $entry_target_path = [System.IO.Path]::Combine($dest, $archive_name)
$entry_dir = [System.IO.Path]::GetDirectoryName($entry_target_path) $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)) { if (-not (Test-Path -LiteralPath $entry_dir)) {
New-Item -Path $entry_dir -ItemType Directory -WhatIf:$check_mode | Out-Null New-Item -Path $entry_dir -ItemType Directory -WhatIf:$check_mode | Out-Null
$result.changed = $true $result.changed = $true

View file

@ -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

View file

@ -1,4 +1,3 @@
---
- name: create test directory - name: create test directory
win_file: win_file:
path: '{{ win_unzip_dir }}\output' path: '{{ win_unzip_dir }}\output'
@ -114,3 +113,59 @@
- unzip_delete is changed - unzip_delete is changed
- unzip_delete.removed - unzip_delete.removed
- not unzip_delete_actual.stat.exists - 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'