Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Fixed security warnings in CVAT server #1246

Closed
wants to merge 5 commits into from

Conversation

nmanovic
Copy link
Contributor

@nmanovic nmanovic commented Mar 9, 2020

Run started:2020-03-09 20:04:49.597572

Test results:
>> Issue: [B314:blacklist] Using xml.etree.ElementTree.iterparse to parse untrusted XML data is known to be vulnerable to XML attacks. Replace xml.etree.ElementTree.iterparse with its defusedxml equivalent function or make sure defusedxml.defuse_stdlib() is called
   Severity: Medium   Confidence: High
   Location: cvat/apps/annotation/cvat.py:429
   More Info: https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html#b313-b320-xml-bad-elementtree
428	    import xml.etree.ElementTree as et
429	    context = et.iterparse(file_object, events=("start", "end"))
430	    context = iter(context)

--------------------------------------------------
>> Issue: [B102:exec_used] Use of exec detected.
   Severity: Medium   Confidence: High
   Location: cvat/apps/annotation/format.py:19
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b102_exec_used.html
18	    }
19	    exec(source_code, global_vars)
20	    if "format_spec" not in global_vars or not isinstance(global_vars["format_spec"], dict):

--------------------------------------------------
>> Issue: [B320:blacklist] Using lxml.etree.fromstring to parse untrusted XML data is known to be vulnerable to XML attacks. Replace lxml.etree.fromstring with its defusedxml equivalent function.
   Severity: Medium   Confidence: High
   Location: cvat/apps/annotation/labelme.py:144
   More Info: https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html#b313-b320-xml-bad-etree
143
144	    root_elem = ET.fromstring(xml_data)
145

--------------------------------------------------
>> Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue.
   Severity: High   Confidence: High
   Location: cvat/apps/auto_annotation/inference_engine.py:17
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html
16	        subprocess.check_output(
17	            'lscpu | grep -o "{}" | head -1'.format(instruction), shell=True
18	        ).decode('utf-8')
19	    )

--------------------------------------------------
>> Issue: [B310:blacklist] Audit url open for permitted schemes. Allowing use of file:/ or custom schemes is often unexpected.
   Severity: Medium   Confidence: High
   Location: cvat/apps/engine/task.py:221
   More Info: https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html#b310-urllib-urlopen
220	        try:
221	            with urlrequest.urlopen(req) as fp, open(os.path.join(upload_dir, name), 'wb') as tfp:
222	                while True:

--------------------------------------------------
>> Issue: [B102:exec_used] Use of exec detected.
   Severity: Medium   Confidence: High
   Location: cvat/apps/engine/utils.py:45
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b102_exec_used.html
44	    try:
45	        exec(source_code, global_vars, local_vars)
46	    except SyntaxError as err:

--------------------------------------------------
>> Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue.
   Severity: High   Confidence: High
   Location: cvat/apps/git/git.py:31
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html
30	    if 'Permission denied' in ex.stderr or 'Could not read from remote repository' in ex.stderr:
31	        keys = subprocess.run(['ssh-add -L'], shell = True,
32	            stdout = subprocess.PIPE).stdout.decode('utf-8').split('\n')
33	        keys = list(filter(len, list(map(lambda x: x.strip(), keys))))

--------------------------------------------------
>> Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue.
   Severity: High   Confidence: High
   Location: cvat/apps/git/git.py:286
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html
285	        if ext == '.zip':
286	            subprocess.call('zip -j -r "{}" "{}"'.format(self._annotation_file, dump_name), shell=True)
287	        elif ext == '.xml':

--------------------------------------------------
>> Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue.
   Severity: High   Confidence: High
   Location: cvat/settings/base.py:54
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html
53	        try:
54	            subprocess.run(['ssh-add {}/*'.format(ssh_dir)], shell = True, stderr = subprocess.PIPE)
55	            keys = subprocess.run(['ssh-add -l'], shell = True,

--------------------------------------------------
>> Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue.
   Severity: High   Confidence: High
   Location: cvat/settings/base.py:55
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html
54	            subprocess.run(['ssh-add {}/*'.format(ssh_dir)], shell = True, stderr = subprocess.PIPE)
55	            keys = subprocess.run(['ssh-add -l'], shell = True,
56	                stdout = subprocess.PIPE).stdout.decode('utf-8').split('\n')
57	            if 'has no identities' in keys[0]:

--------------------------------------------------
>> Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue.
   Severity: High   Confidence: High
   Location: cvat/settings/base.py:62
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html
61	                    print('New pair of keys are being generated')
62	                    subprocess.run(['ssh-keygen -b 4096 -t rsa -f {}/id_rsa -q -N ""'.format(ssh_dir)], shell = True)
63	                    shutil.copyfile('{}/id_rsa'.format(ssh_dir), '{}/id_rsa'.format(keys_dir))

--------------------------------------------------
>> Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue.
   Severity: High   Confidence: High
   Location: cvat/settings/base.py:73
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html
72	                    shutil.copymode('{}/id_rsa.pub'.format(keys_dir), '{}/id_rsa.pub'.format(ssh_dir))
73	                subprocess.run(['ssh-add', '{}/id_rsa'.format(ssh_dir)], shell = True)
74	        finally:

--------------------------------------------------

Code scanned:
	Total lines of code: 10897
	Total lines skipped (#nosec): 0

Run metrics:
	Total issues (by severity):
		Undefined: 0.0
		Low: 31.0
		Medium: 5.0
		High: 7.0
	Total issues (by confidence):
		Undefined: 0.0
		Low: 0.0
		Medium: 0.0
		High: 43.0
Files skipped (0):

@@ -29,7 +29,7 @@

def dump_frame_anno(frame_annotation):
from collections import defaultdict
from lxml import etree as ET
from lxml import etree as ET # nosec (we generate xml)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment makes little sense for unfamiliar reader. Also, it'd be probably better to rephrase it with some accent on that fact of generation, like xml is being generated here.

@nmanovic nmanovic requested a review from azhavoro March 26, 2020 14:03
@nmanovic nmanovic changed the title Fixed security warnings in CVAT server [WIP] Fixed security warnings in CVAT server Mar 31, 2020
@nmanovic
Copy link
Contributor Author

  • Need to add info into CHANGELOG.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants