From d0f7adc5c629475111cdf50bacdeccf247423cf2 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Wed, 24 Jul 2019 16:00:20 -0400 Subject: [PATCH 1/2] prevent templating of passwords from prompt (#59246) * prevent templating of passwords from prompt fixes CVE-2019-10206 (cherry picked from commit e9a37f8e3171105941892a86a1587de18126ec5b) --- .../fragments/dont_template_passwords_from_prompt.yml | 2 ++ lib/ansible/cli/__init__.py | 8 ++++++++ lib/ansible/utils/unsafe_proxy.py | 11 +++++++---- 3 files changed, 17 insertions(+), 4 deletions(-) create mode 100644 changelogs/fragments/dont_template_passwords_from_prompt.yml diff --git a/changelogs/fragments/dont_template_passwords_from_prompt.yml b/changelogs/fragments/dont_template_passwords_from_prompt.yml new file mode 100644 index 000000000000..86a0e6122f94 --- /dev/null +++ b/changelogs/fragments/dont_template_passwords_from_prompt.yml @@ -0,0 +1,2 @@ +bugfixes: + - resolves CVE-2019-10206, by avoiding templating passwords from prompt as it is probable they have special characters. diff --git a/lib/ansible/cli/__init__.py b/lib/ansible/cli/__init__.py index 380ddc4e2a43..76d652f7c8f0 100644 --- a/lib/ansible/cli/__init__.py +++ b/lib/ansible/cli/__init__.py @@ -42,6 +42,7 @@ from ansible.release import __version__ from ansible.utils.path import unfrackpath from ansible.utils.vars import load_extra_vars, load_options_vars +from ansible.utils.unsafe_proxy import AnsibleUnsafeBytes from ansible.vars.manager import VariableManager from ansible.parsing.vault import PromptVaultSecret, get_file_vault_secret @@ -342,6 +343,13 @@ def ask_passwords(self): except EOFError: pass + # we 'wrap' the passwords to prevent templating as + # they can contain special chars and trigger it incorrectly + if sshpass: + sshpass = AnsibleUnsafeBytes(sshpass) + if becomepass: + becomepass = AnsibleUnsafeBytes(becomepass) + return (sshpass, becomepass) def normalize_become_options(self): diff --git a/lib/ansible/utils/unsafe_proxy.py b/lib/ansible/utils/unsafe_proxy.py index 963798a08762..abefc1524914 100644 --- a/lib/ansible/utils/unsafe_proxy.py +++ b/lib/ansible/utils/unsafe_proxy.py @@ -55,7 +55,7 @@ from collections import Mapping, MutableSequence, Set -from ansible.module_utils.six import string_types, text_type +from ansible.module_utils.six import string_types, text_type, binary_type from ansible.module_utils._text import to_text @@ -70,15 +70,18 @@ class AnsibleUnsafeText(text_type, AnsibleUnsafe): pass +class AnsibleUnsafeBytes(binary_type, AnsibleUnsafe): + pass + + class UnsafeProxy(object): def __new__(cls, obj, *args, **kwargs): # In our usage we should only receive unicode strings. # This conditional and conversion exists to sanity check the values # we're given but we may want to take it out for testing and sanitize # our input instead. - if isinstance(obj, string_types): - obj = to_text(obj, errors='surrogate_or_strict') - return AnsibleUnsafeText(obj) + if isinstance(obj, string_types) and not isinstance(obj, AnsibleUnsafeBytes): + obj = AnsibleUnsafeText(to_text(obj, errors='surrogate_or_strict')) return obj From 9f435f433ed5af11801a2b4c4da27ab413914b84 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Wed, 7 Aug 2019 09:11:56 -0500 Subject: [PATCH 2/2] Improve performane of UnsafeProxy __new__ This adds an early return to the __new__ method of the UnsafeProxy object which avoids creating the unsafe object if the incoming object is already unsafe. (cherry picked from commit c1e23c22a9fedafaaa88c2119b26dc123ff1392e) (cherry picked from commit 490f17c7f959ce153765c1f033fdc30becf0faf7) --- lib/ansible/utils/unsafe_proxy.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/ansible/utils/unsafe_proxy.py b/lib/ansible/utils/unsafe_proxy.py index abefc1524914..6221e7339390 100644 --- a/lib/ansible/utils/unsafe_proxy.py +++ b/lib/ansible/utils/unsafe_proxy.py @@ -76,11 +76,17 @@ class AnsibleUnsafeBytes(binary_type, AnsibleUnsafe): class UnsafeProxy(object): def __new__(cls, obj, *args, **kwargs): + if isinstance(obj, AnsibleUnsafe): + # Already marked unsafe + return obj + # In our usage we should only receive unicode strings. # This conditional and conversion exists to sanity check the values # we're given but we may want to take it out for testing and sanitize # our input instead. - if isinstance(obj, string_types) and not isinstance(obj, AnsibleUnsafeBytes): + # Note that this does the wrong thing if we're *intentionall* passing a byte string to this + # function. + if isinstance(obj, string_types): obj = AnsibleUnsafeText(to_text(obj, errors='surrogate_or_strict')) return obj