Develop an automatic test to detect new security flaws in the code #1615

AdriiiPRodri · 2021-03-24T10:58:43Z

Hi team,

In this PR we aim to automate the process of analyzing code for vulnerabilities in order to avoid uploading vulnerable code. This test should be run whenever Python related code is modified, such as the code in the API, Framework and Wodles folders.

ToDo

Create a new unit test that analyzes the Python code and warns about possible bugs.
Investigate if it is possible to detect the code sections changed with respect to the same online branch and launch the tests on them.

Best regards,

Adrián Peña

mcarmona99 · 2021-07-23T15:03:11Z

Issue update

Having into account the investigation done in the issue: wazuh/wazuh#7971, I will try to develop this automatic test using Bandit:

Useful Bandit parameters:

  -i, --confidence      report only issues of a given confidence level or
                        higher (-i for LOW, -ii for MEDIUM, -iii for HIGH)

Testing the framework:

bandit -r framework/ -ii

will show framework vulnerabilities with confidence MEDIUM and HIGH.

These are the issues we are looking for, as we will avoid issues shown by Bandit with lower confidence level. We must ignore vulnerabilities detected in tests as they are not related to the code itself.

The output of this command could be redirected to a temporal file and may be parsed. The problem is the output would be difficult to decode.

Output example for

bandit -r wodles/ -ii :

output.txt

manuel:~/git/wazuh (master)$ bandit -r wodles/ -ii
[main]	INFO	profile include tests: None
[main]	INFO	profile exclude tests: None
[main]	INFO	cli include tests: None
[main]	INFO	cli exclude tests: None
[main]	INFO	running on Python 3.8.10
Run started:2021-07-23 14:55:46.659842

Test results:
>> Issue: [B101:assert_used] Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
   Severity: Low   Confidence: High
   Location: wodles/aws/tests/test_aws.py:63
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b101_assert_used.html
62	
63	        assert(metadata_version == ins.wazuh_version)
64	

--------------------------------------------------
>> Issue: [B101:assert_used] Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
   Severity: Low   Confidence: High
   Location: wodles/aws/tests/test_aws.py:85
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b101_assert_used.html
84	
85	        assert(metadata_version == ins.wazuh_version)
86	

--------------------------------------------------
>> Issue: [B101:assert_used] Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
   Severity: Low   Confidence: High
   Location: wodles/aws/tests/test_aws.py:136
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b101_assert_used.html
135	
136	        assert(data == 8)
137	
138	        # maintenance when retain_db_records is smaller than elements in DB
139	        ins.retain_db_records = 6

...

--------------------------------------------------
>> Issue: [B603:subprocess_without_shell_equals_true] subprocess call - check for execution of untrusted input.
   Severity: Low   Confidence: High
   Location: wodles/oscap/oscap.py:200
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b603_subprocess_without_shell_equals_true.html
199	
200	            ps_xsltproc = Popen([XSLT_BIN, TEMPLATE_OVAL, FIFO_PATH], stdin=None, stdout=PIPE, stderr=STDOUT)
201	            ps.wait()

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

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

Run metrics:
	Total issues (by severity):
		Undefined: 0.0
		Low: 16.0
		Medium: 5.0
		High: 1.0
	Total issues (by confidence):
		Undefined: 0.0
		Low: 0.0
		Medium: 0.0
		High: 22.0
Files skipped (0):

I am investigating Bandit report formatters that could be useful for the output that will generate our test: https://bandit.readthedocs.io/en/latest/formatters/index.html

The format could be: csv, custom, html, json, screen, text, xml, yaml.

In our case, the most interesting ones may be json and html.

mcarmona99 · 2021-07-26T15:12:52Z

Issue update

Useful Bandit parameters:

  -i, --confidence      report only issues of a given confidence level or
                        higher (-i for LOW, -ii for MEDIUM, -iii for HIGH)
                        
  -r, --recursive       find and process files in subdirectories
  
  -f {csv,custom,html,json,screen,txt,xml,yaml}, --format {csv,custom,html,json,screen,txt,xml,yaml}
                        specify output format
                        
  -x EXCLUDED_PATHS, --exclude EXCLUDED_PATHS
                        comma-separated list of paths (glob patterns
                        supported) to exclude from scan (note that these are
                        in addition to the excluded paths provided in the
                        config file)

Bandit can create html reports with the -f flag:

bandit -r wodles/ -ii -f html

Reports like the folllowing can be created, they will be useful for a future integration in our automation environment:

We will use -x to exclude tests.

In the test that is going to be created in this issue, no html format will be done as we need a json output to assert errors.

The command that will be used in our test will be the following:

bandit -r wodles|framework|api -ii -f json -x tests/,test/

That command will generate a JSON report whith the possible vulnerabilities detected by Bandit.

The test will pass if the detected vulnerabilities are 0. As investigated in the issue wazuh/wazuh#7998, there are some vulnerabilities that could be ignored, as they are not real vulnerabilities.

Vulnerabilities will never be 0 because of this false positives. In order to ignore them, this test will compare all the vulnerabilities detected with a list of known vulnerabilities (false positives and vulnerabilities to fix).

If there are new vulnerabilities, the test will fail and we will have to study whether the possible flaws are dangerous or not. In case they are false positives, we will add them to the known_flaws list (that will be a simple txt file).

When fixing a known vulnerability or flaw, we will remove its entry in the respective known_flaws list.

mcarmona99 · 2021-09-10T11:51:45Z

API - Possible vulnerabilities

Total of possible flaws in api/: 2

`wazuh-apid.py:`

Code:

    app.add_api('spec.yaml',
                arguments={'title': 'Wazuh API',
                           'protocol': 'https' if api_conf['https']['enabled'] else 'http',
                           'host': api_conf['host'],
                           'port': api_conf['port']
                           },
                strict_validation=True,
                validate_responses=False,
                pass_context_arg_name='request',
                options={"middlewares": [response_postprocessing, set_user_name, security_middleware, request_logging,
                                         set_secure_headers]})

Possible flaw: Possible hardcoded password: 'request'

`configuration.py`

Code:

default_api_configuration = {
    "host": "0.0.0.0",
    "port": 55000,
    "use_only_authd": False,
    "drop_privileges": True,
    "experimental_features": False,
    "max_upload_size": 10485760,
    "intervals": {
        "request_timeout": 10
    },
    "https": {
        "enabled": True,
        "key": "api/configuration/ssl/server.key",
        "cert": "api/configuration/ssl/server.crt",
        "use_ca": False,
        "ca": "api/configuration/ssl/ca.crt",
        "ssl_protocol": "TLSv1.2",
        "ssl_ciphers": ""
    },
    "logs": {
        "level": "info",
        "path": "logs/api.log"
    },
    "cors": {
        "enabled": False,
        "source_route": "*",
        "expose_headers": "*",
        "allow_headers": "*",
        "allow_credentials": False,
    },
    "cache": {
        "enabled": True,
        "time": 0.750
    },
    "access": {
        "max_login_attempts": 50,
        "block_time": 300,
        "max_request_per_minute": 300
    },
    "remote_commands": {
        "localfile": {
            "enabled": True,
            "exceptions": []
        },
        "wodle_command": {
            "enabled": True,
            "exceptions": []
        }
    }
}

Possible flaw: Possible binding to all interfaces.

In order to investigate these possible flaws, I have opened the issue wazuh/wazuh#10142

mcarmona99 · 2021-09-14T07:06:50Z

Framework - Possible vulnerabilities

Total of possible flaws in framework/: 28

Subprocess calls (11 flaws)

All of the flaws are due to the use of subprocess. The last 5 are also because of the use of subprocess but the difference is that these possible flaws were just telling to consider the possible security implications of importing subprocess, instead of its usage.

In order to investigate the possible vulnerabilities, the following issue was opened: wazuh/wazuh#10144

XML calls (2 flaws)

Issue text: Using tostring to parse untrusted XML data is known to be vulnerable to XML attacks. Replace tostring with the equivalent defusedxml package, or make sure defusedxml.defuse_stdlib() is called.

framework/wazuh/core/configuration.py line 15

Issue text: Using ElementTree to parse untrusted XML data is known to be vulnerable to XML attacks. Replace ElementTree with the equivalent defusedxml package, or make sure defusedxml.defuse_stdlib() is called.

framework/wazuh/core/utils.py line 25

As bandit indicates, in this case it is always convenient to use defusedxml because it does not reduce functionality and increases security.
These flaws can be avoided with the use of defusedxml (we have already used it in other situations, so we have the dep in the requirements.txt).

Issue opened and flaws reported. wazuh/wazuh#10113

Chmod setting a permissive mask (2 flaws)

Issue text: Chmod setting a permissive mask 0o770 on file (group_path).

framework/wazuh/agent.py line 582.

Issue text: Chmod setting a permissive mask 0o750 on file (agent_backup_dir).

framework/wazuh/core/agent.py line 611.

A MEDIUM warning is generated if a file is set to group executable and a HIGH warning is reported if a file is set world writable. In this case, both possible flaws are reported with severity MEDIUM so the file is set to group executable.

This should not be considered a flaw as it is a Wazuh functional requirement.

Issue opened and flaws reported. wazuh/wazuh#10145

Use of insecure MD2, MD4, MD5, or SHA1 hash function (3 flaws)

Create issue to investigate if it is possible to use SHA256 or a different secure cryptograpthic algorithm.
Algorithms which are not considered secure with alternatives: https://security.stackexchange.com/questions/78/what-cryptographic-algorithms-are-not-considered-secure

Issue opened and flaws reported. wazuh/wazuh#10114

Possible binding to all interfaces (2 flaws)

This flaw told us that we are exposing something from all network interfaces.

framework/wazuh/core/cluster/cluster.py line 64

reservated_ips = {'localhost', 'NODE_IP', '0.0.0.0', '127.0.1.1'}
...
invalid_elements = list(reservated_ips & set(config['nodes']))

framework/wazuh/core/cluster/utils.py line 48-57

Bandit is reporting this:

cluster_default_configuration = {
        'disabled': False,
        'node_type': 'master',
        'name': 'wazuh',
        'node_name': 'node01',
        'key': '',
        'port': 1516,
        'bind_addr': '0.0.0.0',
        'nodes': ['NODE_IP'],
        'hidden': 'no'
    }

as a possible binding from all interfaces. This may not be a real flaw as this is part of the design of the cluster.

Issue opened: wazuh/wazuh#10146

Probable insecure usage of temp file/directory (2 flaws)

This is a hardcoded temporary directory. We could investigate if we could generate a temporary one in a safer way (the tmpfile library can create it by its own): https://docs.openstack.org/bandit/1.4.0/plugins/hardcoded_tmp_directory.html

Issue opened and flaws reported. wazuh/wazuh#10115

Possible SQL injection vector through string-based query construction (5 flaws)

Issue opened to investigate this flaw: wazuh/wazuh#10147

Pseudo-random generators (1 flaw)

Issue text: Standard pseudo-random generators are not suitable for security/cryptographic purposes

framework/wazuh/core/cluster/cluster.py line 306

This random number is used to set the name of the compressed data for the integrity synchronization process in cluster. The temporary file is created using the node name, so we can guaranteee that the files will never be overwritten.

Despite this, an issue has been created to investigate it: wazuh/wazuh#10154

mcarmona99 · 2021-09-14T07:32:05Z

Wazuh modules - Possible vulnerabilities

Total of possible flaws in wodles/: 13

Standard pseudo-random generators (1 flaw)

The flaw:

Standard pseudo-random generators are not suitable for security/cryptographic purposes.

wodles/oscap/oscap.py line 164

Has already been studied in wazuh/wazuh#7998 and an issue was opened to investigate and fix it. The issue opened is wazuh/wazuh#8347.

Use of insecure MD2, MD4, MD5, or SHA1 hash function (3 flaws)

This is not a problem since the data handled are not critical and are only used internally by the wodle, so there is no decoding possibility. Despite this, we could open an issue to investigate and use secure hash functions, similar to the same possible flaws found in framework.

Issue opened: wazuh/wazuh#10116

Subprocess calls (7 flaws)

The issue wazuh/wazuh#8347 was opened to investigate and avoid these subprocess calls.

Diferent to the subprocess calls found in framework, now we can see that we make subprocess calls with command line arguments. Example:

profiles = extract_profiles_from_file(arg_file)

arg_file is created from argv and the function extract_profiles_from_file uses subprocess.

Use of eval: insecure function (2 flaws)

Issue text: Use of possibly insecure function - consider using safer ast.literal_eval.

This was also reported in wazuh/wazuh#8347 as we should consider using a safer function instead of eval.

Try, Except, Pass detected (1 flaw)

Issue text: Try, Except, Pass detected.

wodles/utils.py line 47-48

Issue opened to investigate this possible vulnerability: wazuh/wazuh#10155

mcarmona99 · 2021-09-16T08:56:34Z

Issue update - Summary

To sum up, I have created 12 new issues reporting the possible vulnerabilities found in the wazuh repository in its actual master branch. The parameters used in the test have been the default ones.

These issues have been grouped in an epic one: wazuh/wazuh#10125

davidjiglesias changed the title ~~Develop an automatic test to detect new security flaws in the code.~~ Develop an automatic test to detect new security flaws in the code Jul 16, 2021

AdriiiPRodri transferred this issue from wazuh/wazuh Jul 19, 2021

AdriiiPRodri added feature/api feature/framework team/framework labels Jul 19, 2021

mcarmona99 self-assigned this Jul 22, 2021

mcarmona99 mentioned this issue Jul 27, 2021

Add test to check new possible flaws in wodles, framework and API code #1659

Merged

6 tasks

This was referenced Sep 16, 2021

Fix findings found after the analysis of the Wodles code wazuh/wazuh#8347

Closed

Investigate and fix possible vulnerability when using Try, Except, Pass in wodles/ wazuh/wazuh#10155

Closed

Kondent mentioned this issue Sep 16, 2021

Develop an automatic test/tool to detect new security flaws in the embedded python or dependencies #1612

Closed

snaow closed this as completed in #1659 Oct 1, 2021

Kondent mentioned this issue Nov 2, 2021

Fix random values at cluster zip filenames to be cryptographically secure wazuh/wazuh#10710

Merged

mcarmona99 mentioned this issue Dec 13, 2021

Review Try, Except, Pass used in wazuh-clusterd.py wazuh/wazuh#11343

Closed

13 tasks

Kondent mentioned this issue Feb 2, 2022

Investigate possible vulnerabilities when using subprocess in the wodles/ directory wazuh/wazuh#12067

Closed

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Develop an automatic test to detect new security flaws in the code #1615

Develop an automatic test to detect new security flaws in the code #1615

AdriiiPRodri commented Mar 24, 2021 •

edited by mcarmona99

Loading

mcarmona99 commented Jul 23, 2021

mcarmona99 commented Jul 26, 2021

mcarmona99 commented Sep 10, 2021 •

edited

Loading

mcarmona99 commented Sep 14, 2021 •

edited

Loading

mcarmona99 commented Sep 14, 2021 •

edited

Loading

mcarmona99 commented Sep 16, 2021 •

edited

Loading

Develop an automatic test to detect new security flaws in the code #1615

Develop an automatic test to detect new security flaws in the code #1615

Comments

AdriiiPRodri commented Mar 24, 2021 • edited by mcarmona99 Loading

ToDo

mcarmona99 commented Jul 23, 2021

Issue update

mcarmona99 commented Jul 26, 2021

Issue update

mcarmona99 commented Sep 10, 2021 • edited Loading

API - Possible vulnerabilities

wazuh-apid.py:

configuration.py

mcarmona99 commented Sep 14, 2021 • edited Loading

Framework - Possible vulnerabilities

Subprocess calls (11 flaws)

XML calls (2 flaws)

Chmod setting a permissive mask (2 flaws)

Use of insecure MD2, MD4, MD5, or SHA1 hash function (3 flaws)

Possible binding to all interfaces (2 flaws)

Probable insecure usage of temp file/directory (2 flaws)

Possible SQL injection vector through string-based query construction (5 flaws)

Pseudo-random generators (1 flaw)

mcarmona99 commented Sep 14, 2021 • edited Loading

Wazuh modules - Possible vulnerabilities

Standard pseudo-random generators (1 flaw)

Use of insecure MD2, MD4, MD5, or SHA1 hash function (3 flaws)

Subprocess calls (7 flaws)

Use of eval: insecure function (2 flaws)

Try, Except, Pass detected (1 flaw)

mcarmona99 commented Sep 16, 2021 • edited Loading

Issue update - Summary

AdriiiPRodri commented Mar 24, 2021 •

edited by mcarmona99

Loading

mcarmona99 commented Sep 10, 2021 •

edited

Loading

`wazuh-apid.py:`

`configuration.py`

mcarmona99 commented Sep 14, 2021 •

edited

Loading

mcarmona99 commented Sep 14, 2021 •

edited

Loading

mcarmona99 commented Sep 16, 2021 •

edited

Loading