Cannot detect "else:" condition in "choice.yaml"

[watanabemk]

ENVIRONMENT

OS and Version: Ubuntu 22.04.3 LTS (Jammy Jellyfish) on WSL2
Python Version: 3.10.12
MobSF Version: v3.7.9 beta

EXPLANATION OF THE ISSUE

Detection patterns like "-id: rule3" in "choice.yaml" are not working.
The "else:" condition in "choice_matcher.py" may not be working.

STEPS TO REPRODUCE THE ISSUE

Upload the Android app "AndroGoa.apk" to MobSF.
"NIAP ANALYSIS v1.3" only detects 10 locations
*MobSF Version: 13 locations detected in v3.7.6

ADDITIONAL INFORMATION

It may be cured by the following method.
current situation

def add_finding(self, results):
    """Add Choice Findings."""
    for res_list in results:
        if not res_list:
            continue
        for match_dict in res_list:
            all_matches = match_dict['all_matches']
            matches = match_dict['matches']
            rule = match_dict['rule']
            if all_matches:
                selection = rule['selection'].format(list(all_matches))
            elif matches:
                select = rule['choice'][min(matches)][1]
                selection = rule['selection'].format(select)
            elif rule.get('else'):
                selection = rule['selection'].format(rule['else'])
            else:
                continue
            self.findings[rule['id']] = self.get_meta(rule, selection)

Potential Issues

1. Evaluation of all_matches and matches:
   all_matches and matches are evaluated as False even when they are empty, which should lead to the else condition being executed if there are no matching items.
   However, if all_matches or matches are empty sets or lists, both elif matches: and elif rule.get('else'): may be evaluated as False, preventing the else condition from being executed.

2. Placement of elif Conditions:
   The placement of elif rule.get('else'): after all_matches and matches might lead to situations where the else condition is not appropriately evaluated, even when they are empty.

3. Use of continue:
   The continue statement following the else block is used to move to the next iteration if rule.get('else') is False (i.e., the else key doesn't exist). However, this might lead to scenarios where the else key exists but is still skipped.
   Improvement proposal

def add_finding(self, results):
    """Add Choice Findings."""
    for res_list in results:
        if not res_list:
            continue
        for match_dict in res_list:
            all_matches = match_dict['all_matches']
            matches = match_dict['matches']
            rule = match_dict['rule']

            # Check the else condition if all_matches and matches are empty
            if all_matches:
                selection = rule['selection'].format(list(all_matches))
            elif matches:
                select = rule['choice'][min(matches)][1]
                selection = rule['selection'].format(select)
            else:
                # Use the else condition if both all_matches and matches are empty
                selection = rule['selection'].format(rule.get('else', ''))

            self.findings[rule['id']] = self.get_meta(rule, selection)

With this change, the else condition will be properly evaluated when both all_matches and matches are empty, ensuring the operation works as expected.

LOG FILE

none

[ajinabraham]

addressed