Fix nxos_file_copy option value path validation (#65423)

* Fix nxos_file_copy option value path validation

*  Modify `local_file`, `local_file_directory` and
   `remote_file` option type from `str` to `path`
   so that the option value is validated in Ansible
   for a legitimate path value

* Fix review comments

changelogs/fragments/nxos_file_copy_path_issue.yml

@@ -0,0 +1,6 @@
+bugfixes:
+- "CVE-2019-14905 - nxos_file_copy module accepts remote_file parameter which is used for destination name
+   and performs actions related to that on the device using the value of remote_file which is of string type
+   However, there is no user input validation done while performing actions. A malicious code could crafts
+   the filename parameter to take advantage by performing an OS command injection. This fix validates the
+   option value if it is legitimate file path or not."

lib/ansible/modules/network/nxos/nxos_file_copy.py

@@ -151,7 +151,6 @@ EXAMPLES = '''
 # Initiate file copy from the nxos device to transfer file from an SCP server back to the nxos device
   - name: "initiate file copy from device"
     nxos_file_copy:
-      nxos_file_copy:
       file_pull: True
       local_file: "xyz"
       local_file_directory: "dir1/dir2/dir3"

lib/ansible/plugins/action/nxos_file_copy.py

@@ -24,7 +24,8 @@ import re
 import time
 
 from ansible.errors import AnsibleError
-from ansible.module_utils._text import to_text, to_bytes
+from ansible.module_utils._text import to_text, to_bytes, to_native
+from ansible.module_utils.common import validation
 from ansible.module_utils.connection import Connection
 from ansible.plugins.action import ActionBase
 from ansible.utils.display import Display
@@ -58,9 +59,9 @@ class ActionModule(ActionBase):
             file_pull_timeout=dict(type='int', default=300),
             file_pull_compact=dict(type='bool', default=False),
             file_pull_kstack=dict(type='bool', default=False),
-            local_file=dict(type='str'),
-            local_file_directory=dict(type='str'),
-            remote_file=dict(type='str'),
+            local_file=dict(type='path'),
+            local_file_directory=dict(type='path'),
+            remote_file=dict(type='path'),
             remote_scp_server=dict(type='str'),
             remote_scp_server_user=dict(type='str'),
             remote_scp_server_password=dict(no_log=True),
@@ -72,24 +73,23 @@ class ActionModule(ActionBase):
             playvals[key] = self._task.args.get(key, argument_spec[key].get('default'))
             if playvals[key] is None:
                 continue
-            if argument_spec[key].get('type') is None:
-                argument_spec[key]['type'] = 'str'
-            type_ok = False
-            type = argument_spec[key]['type']
-            if type == 'str':
-                if isinstance(playvals[key], six.string_types):
-                    type_ok = True
-            elif type == 'int':
-                if isinstance(playvals[key], int):
-                    type_ok = True
-            elif type == 'bool':
-                if isinstance(playvals[key], bool):
-                    type_ok = True
-            else:
-                raise AnsibleError('Unrecognized type <{0}> for playbook parameter <{1}>'.format(type, key))
 
-            if not type_ok:
-                raise AnsibleError('Playbook parameter <{0}> value should be of type <{1}>'.format(key, type))
+            option_type = argument_spec[key].get('type', 'str')
+            try:
+                if option_type == 'str':
+                    playvals[key] = validation.check_type_str(playvals[key])
+                elif option_type == 'int':
+                    playvals[key] = validation.check_type_int(playvals[key])
+                elif option_type == 'bool':
+                    playvals[key] = validation.check_type_bool(playvals[key])
+                elif option_type == 'path':
+                    playvals[key] = validation.check_type_path(playvals[key])
+                else:
+                    raise AnsibleError('Unrecognized type <{0}> for playbook parameter <{1}>'.format(type, key))
+
+            except (TypeError, ValueError) as e:
+                raise AnsibleError("argument %s is of type %s and we were unable to convert to %s: %s"
+                                   % (key, type(playvals[key]), option_type, to_native(e)))
 
         # Validate playbook dependencies
         if playvals['file_pull']:

test/integration/targets/nxos_file_copy/tests/cli/input_validation.yaml

@@ -1,15 +1,16 @@
 ---
 - debug: msg="START nxos_file_copy input_validation test"
 
-- name: "Input Validation - param should be type <str>"
+- name: "Input Validation - param should be type <path>"
   nxos_file_copy:
     remote_file: 500
+    file_pull: True
   register: result
   ignore_errors: true
 
 - assert:
     that:
-      - result is search('Playbook parameter <remote_file> value should be of type <str>')
+      - not result is search('argument remote_file is of type')
 
 - name: "Input Validation - param should be type <int>"
   nxos_file_copy:
@@ -19,7 +20,7 @@
 
 - assert:
     that:
-      - result is search('Playbook parameter <file_pull_timeout> value should be of type <int>')
+      - result is search("argument file_pull_timeout is of type <class 'ansible.parsing.yaml.objects.AnsibleUnicode'> and we were unable to convert to int")
 
 - name: "Input Validation - param should be type <bool>"
   nxos_file_copy:
@@ -29,7 +30,7 @@
 
 - assert:
     that:
-      - result is search('Playbook parameter <file_pull> value should be of type <bool>')
+      - result is search("argument file_pull is of type <class 'ansible.parsing.yaml.objects.AnsibleUnicode'> and we were unable to convert to bool")
 
 - name: "Input Validation - param <file_pull> <remote_file> dependency"
   nxos_file_copy:
@@ -39,7 +40,7 @@
 
 - assert:
     that:
-      - result is search('Playbook parameter <remote_file> required when <file_pull> is True')
+      - result is search("Playbook parameter <remote_file> required when <file_pull> is True")
 
 - name: "Input Validation - param <file_pull> <remote_scp_server> dependency"
   nxos_file_copy: