prevent ansible_facts injection (#68431)
- also only replace when needed - switched from replace to index - added test to verify bogus_facts are not accepted CVE-2020-10684
This commit is contained in:
parent
97e5179745
commit
a9d2ceafe4
7 changed files with 36 additions and 6 deletions
2
changelogs/fragments/af_clean.yml
Normal file
2
changelogs/fragments/af_clean.yml
Normal file
|
@ -0,0 +1,2 @@
|
||||||
|
bugfixes:
|
||||||
|
- Ensure we don't allow ansible_facts subkey of ansible_facts to override top level, also fix 'deprefixing' to prevent key transforms.
|
|
@ -112,7 +112,7 @@ MODULE_REQUIRE_ARGS = ('command', 'win_command', 'ansible.windows.win_command',
|
||||||
'ansible.windows.win_shell', 'raw', 'script')
|
'ansible.windows.win_shell', 'raw', 'script')
|
||||||
MODULE_NO_JSON = ('command', 'win_command', 'ansible.windows.win_command', 'shell', 'win_shell',
|
MODULE_NO_JSON = ('command', 'win_command', 'ansible.windows.win_command', 'shell', 'win_shell',
|
||||||
'ansible.windows.win_shell', 'raw')
|
'ansible.windows.win_shell', 'raw')
|
||||||
RESTRICTED_RESULT_KEYS = ('ansible_rsync_path', 'ansible_playbook_python')
|
RESTRICTED_RESULT_KEYS = ('ansible_rsync_path', 'ansible_playbook_python', 'ansible_facts')
|
||||||
TREE_DIR = None
|
TREE_DIR = None
|
||||||
VAULT_VERSION_MIN = 1.0
|
VAULT_VERSION_MIN = 1.0
|
||||||
VAULT_VERSION_MAX = 1.0
|
VAULT_VERSION_MAX = 1.0
|
||||||
|
|
|
@ -16,7 +16,6 @@ from ansible.module_utils.common._collections_compat import MutableMapping, Muta
|
||||||
from ansible.plugins.loader import connection_loader
|
from ansible.plugins.loader import connection_loader
|
||||||
from ansible.utils.display import Display
|
from ansible.utils.display import Display
|
||||||
|
|
||||||
|
|
||||||
display = Display()
|
display = Display()
|
||||||
|
|
||||||
|
|
||||||
|
@ -169,10 +168,9 @@ def namespace_facts(facts):
|
||||||
''' return all facts inside 'ansible_facts' w/o an ansible_ prefix '''
|
''' return all facts inside 'ansible_facts' w/o an ansible_ prefix '''
|
||||||
deprefixed = {}
|
deprefixed = {}
|
||||||
for k in facts:
|
for k in facts:
|
||||||
if k in ('ansible_local',):
|
if k.startswith('ansible_') and k not in ('ansible_local',):
|
||||||
# exceptions to 'deprefixing'
|
deprefixed[k[8:]] = module_response_deepcopy(facts[k])
|
||||||
deprefixed[k] = module_response_deepcopy(facts[k])
|
|
||||||
else:
|
else:
|
||||||
deprefixed[k.replace('ansible_', '', 1)] = module_response_deepcopy(facts[k])
|
deprefixed[k] = module_response_deepcopy(facts[k])
|
||||||
|
|
||||||
return {'ansible_facts': deprefixed}
|
return {'ansible_facts': deprefixed}
|
||||||
|
|
12
test/integration/targets/gathering_facts/library/bogus_facts
Normal file
12
test/integration/targets/gathering_facts/library/bogus_facts
Normal file
|
@ -0,0 +1,12 @@
|
||||||
|
#!/bin/sh
|
||||||
|
|
||||||
|
echo '{
|
||||||
|
"changed": false,
|
||||||
|
"ansible_facts": {
|
||||||
|
"ansible_facts": {
|
||||||
|
"discovered_interpreter_python": "(touch /tmp/pwned-$(date -Iseconds)-$(whoami) ) 2>/dev/null >/dev/null && /usr/bin/python",
|
||||||
|
"bogus_overwrite": "yes"
|
||||||
|
},
|
||||||
|
"dansible_iscovered_interpreter_python": "(touch /tmp/pwned-$(date -Iseconds)-$(whoami) ) 2>/dev/null >/dev/null && /usr/bin/python"
|
||||||
|
}
|
||||||
|
}'
|
|
@ -7,3 +7,6 @@ ansible-playbook test_gathering_facts.yml -i inventory -v "$@"
|
||||||
# ANSIBLE_CACHE_PLUGIN=base ansible-playbook test_gathering_facts.yml -i inventory -v "$@"
|
# ANSIBLE_CACHE_PLUGIN=base ansible-playbook test_gathering_facts.yml -i inventory -v "$@"
|
||||||
|
|
||||||
ANSIBLE_GATHERING=smart ansible-playbook test_run_once.yml -i inventory -v "$@"
|
ANSIBLE_GATHERING=smart ansible-playbook test_run_once.yml -i inventory -v "$@"
|
||||||
|
|
||||||
|
# ensure clean_facts is working properly
|
||||||
|
ansible-playbook test_prevent_injection.yml -i inventory -v "$@"
|
||||||
|
|
|
@ -0,0 +1,14 @@
|
||||||
|
- name: Ensure clean_facts is working properly
|
||||||
|
hosts: facthost1
|
||||||
|
gather_facts: false
|
||||||
|
tasks:
|
||||||
|
- name: gather 'bad' facts
|
||||||
|
action: bogus_facts
|
||||||
|
|
||||||
|
- name: ensure that the 'bad' facts didn't polute what they are not supposed to
|
||||||
|
assert:
|
||||||
|
that:
|
||||||
|
- "'touch' not in discovered_interpreter_python|default('')"
|
||||||
|
- "'touch' not in ansible_facts.get('discovered_interpreter_python', '')"
|
||||||
|
- "'touch' not in ansible_facts.get('ansible_facts', {}).get('discovered_interpreter_python', '')"
|
||||||
|
- bogus_overwrite is undefined
|
|
@ -269,6 +269,7 @@ test/integration/targets/collections_relative_imports/collection_root/ansible_co
|
||||||
test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/modules/my_module.py pylint:relative-beyond-top-level
|
test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/modules/my_module.py pylint:relative-beyond-top-level
|
||||||
test/integration/targets/expect/files/test_command.py future-import-boilerplate
|
test/integration/targets/expect/files/test_command.py future-import-boilerplate
|
||||||
test/integration/targets/expect/files/test_command.py metaclass-boilerplate
|
test/integration/targets/expect/files/test_command.py metaclass-boilerplate
|
||||||
|
test/integration/targets/gathering_facts/library/bogus_facts shebang
|
||||||
test/integration/targets/get_url/files/testserver.py future-import-boilerplate
|
test/integration/targets/get_url/files/testserver.py future-import-boilerplate
|
||||||
test/integration/targets/get_url/files/testserver.py metaclass-boilerplate
|
test/integration/targets/get_url/files/testserver.py metaclass-boilerplate
|
||||||
test/integration/targets/group/files/gidget.py future-import-boilerplate
|
test/integration/targets/group/files/gidget.py future-import-boilerplate
|
||||||
|
|
Loading…
Reference in a new issue