Bug: Multiple instances of rule options fields clobber eachother

[andrewbolster]

As per snort docs:

Note that multiple content rules can be specified in one rule. This allows rules to be tailored for less false positives.

Example ET Rule with multiple contents:

alert tcp $EXTERNAL_NET 443 -> $HOME_NET any (msg:"ET CURRENT_EVENTS Possible Upatre SSL Cert directory92.com"; flow:established,from_server; content:"|55 04 03|"; content:"|0f|directory92.com"; distance:1; within:16; reference:md5,dc7939920cb93e58c990a8e0a0295bb7; classtype:trojan-activity; sid:2019027; tag:session,5,packets; rev:1; metadata:affected_product Windows_XP_Vista_7_8_10_Server_32_64_Bit, attack_target Client_Endpoint, deployment Perimeter, tag SSL_Malicious_Cert, tag Exploit_Kit, tag Downloader, tag Upatre, signature_severity Critical, created_at 2014_08_27, malware_family Upatre, updated_at 2016_07_01;)

rule.parse output (with highlighting)

{'enabled': True,
 'action': 'alert',
 'direction': '->',
 'group': None,
 'gid': 1,
 'sid': 2019027,
 'rev': 1,
 'msg': 'ET CURRENT_EVENTS Possible Upatre SSL Cert directory92.com',
 'flowbits': [],
 'metadata': ['affected_product Windows_XP_Vista_7_8_10_Server_32_64_Bit',
  'attack_target Client_Endpoint',
  'deployment Perimeter',
  'tag SSL_Malicious_Cert',
  'tag Exploit_Kit',
  'tag Downloader',
  'tag Upatre',
  'signature_severity Critical',
  'created_at 2014_08_27',
  'malware_family Upatre',
  'updated_at 2016_07_01'],
 'references': ['md5,dc7939920cb93e58c990a8e0a0295bb7'],
 'classtype': 'trojan-activity',
 'priority': 0,
 'options': [{'name': 'msg',
   'value': '"ET CURRENT_EVENTS Possible Upatre SSL Cert directory92.com"'},
  {'name': 'flow', 'value': 'established,from_server'},
  {'name': 'content', 'value': '"|55 04 03|"'},
  {'name': 'content', 'value': '"|0f|directory92.com"'},
  {'name': 'distance', 'value': '1'},
  {'name': 'within', 'value': '16'},
  {'name': 'reference', 'value': 'md5,dc7939920cb93e58c990a8e0a0295bb7'},
  {'name': 'classtype', 'value': 'trojan-activity'},
  {'name': 'sid', 'value': '2019027'},
  {'name': 'tag', 'value': 'session,5,packets'},
  {'name': 'rev', 'value': '1'},
  {'name': 'metadata',
   'value': 'affected_product Windows_XP_Vista_7_8_10_Server_32_64_Bit, attack_target Client_Endpoint, deployment Perimeter, tag SSL_Malicious_Cert, tag Exploit_Kit, tag Downloader, tag Upatre, signature_severity Critical, created_at 2014_08_27, malware_family Upatre, updated_at 2016_07_01'}],
 'raw': 'alert tcp $EXTERNAL_NET 443 -> $HOME_NET any (msg:"ET CURRENT_EVENTS Possible Upatre SSL Cert directory92.com"; flow:established,from_server; content:"|55 04 03|"; content:"|0f|directory92.com"; distance:1; within:16; reference:md5,dc7939920cb93e58c990a8e0a0295bb7; classtype:trojan-activity; sid:2019027; tag:session,5,packets; rev:1; metadata:affected_product Windows_XP_Vista_7_8_10_Server_32_64_Bit, attack_target Client_Endpoint, deployment Perimeter, tag SSL_Malicious_Cert, tag Exploit_Kit, tag Downloader, tag Upatre, signature_severity Critical, created_at 2014_08_27, malware_family Upatre, updated_at 2016_07_01;)',
 'header': 'alert tcp $EXTERNAL_NET 443 -> $HOME_NET any',
 'flow': 'established,from_server',
*'content': '"|0f|directory92.com"',*
 'distance': '1',
 'within': '16',
 'tag': 'session,5,packets'}

While the correct content data is extractable from the options object, this may be misleading, particularly in bulk-rule-processing (as I was doing).

Suggested fixes

Either:

Make content a list to reflect the specification and add a specific elif name = 'content' options->rule loop in parse

Or:

Prevent clobbering in the same loop by doing a generic name existence check and listifying+appending if the name already exists in the rule

[sevdog]

I dont get the use case in which this could be misleading or considered as a bug.

Could you please explain this?

[andrewbolster]

In the example above the signature has multiple content fields;

 {'name': 'content', 'value': '"|55 04 03|"'},
 {'name': 'content', 'value': '"|0f|directory92.com"'},

But the parsed rule object only presents the last one; {'content':'"|0f|directory92.com"'},

[sevdog]

IMO its some kind of unwanted bug caused by this line of code:

# omissis
else:
    rule[name] = val
# omissis

If you better inspect the parsed rule you can also note these other values which do not make sense in that place:

 'distance': '1',
 'within': '16',
 'tag': 'session,5,packets'

If you need to check for content options its better to inspect the options key.

A fix can be done easily by removing that line of code.

Maybe @jasonish this could be pointed out also in suricata-update.

[jasonish]

Yes, the fix will be to remove the field from the decode rules and loop through the options.

This stuff has been removed from suricata-update already as its not relevant to that app.