Project

General

Profile

Actions

Bug #1638

closed

rule parsing issues: rev

Added by Victor Julien over 8 years ago. Updated about 8 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

The rule parser crashes on certain bad rules.

Rules:

alert http any any -> any any (content:"abc"; http_uri; sid:1; rev:";)
alert http any !ny -> any any (content:"abc"; http_uri; sid:1; rev:";)
alert http any any -> any !ny (content:"abc"; http_uri; sid:1; rev:";)
alert http any any <> any any (content:"abc"; http_uri; sid:1; rev:";)
alert http any any -> any any (content:"abc"; http_uri;  id:1; rev:";)
alert http any any -> any any (content:"abc";id:1; rev:";)
alert http any !ny -> any !ny (content:"abc"; http_uri; sid:1; rev:";)
alert http any :ny -> any any (content:"abc"; http_uri; sid:1; rev:";)
alert http any any <> any any (content:"abc"; http_uri; gid:1; rev:";)
alert http any any <> any any (content:"abc"; http_uri; rev:"; r;)
alert http any any <> any any (content:"abc"; http_uri; gid:1; rev:";)
alert http any !ny -> any !ny (id:1; rev:";)

Found by AFL.

To reproduce, compile Suricata with ASAN and simply run:

suricata -c suricata.yaml -S <rulefile> -T

Actions #1

Updated by Victor Julien over 8 years ago

alert http any any -> any any (content:"abc"; http_uri; sid:1; rev:";)
alert http any !ny -> any any (content:"abc"; http_uri; sid:1; rev:";)
alert http any any -> any !ny (content:"abc"; http_uri; sid:1; rev:";)
alert http any any <> any any (content:"abc"; http_uri; sid:1; rev:";)
alert http any any -> any any (content:"abc"; http_uri;  id:1; rev:";)
alert http any any -> any any (content:"abc";id:1; rev:";)
alert http any !ny -> any !ny (content:"abc"; http_uri; sid:1; rev:";)
alert http any :ny -> any any (content:"abc"; http_uri; sid:1; rev:";)
alert http any any <> any any (content:"abc"; http_uri; gid:1; rev:";)
alert http any any <> any any (content:"abc"; http_uri; rev:"; r;)
alert http any any <> any any (content:"abc"; http_uri; gid:1; rev:";)
alert http any !ny -> any !ny (id:1; rev:";)
alert http any any <> any 1ny (content:"abc"; content:"abc"; http_uri; sid:1; rev:";)
Actions #2

Updated by Victor Julien over 8 years ago

alert http any !ny -> any ,ny (content:"abc"; http_uri; sid:1; rev:";)
alert http any an, <> any any (content:"abcrt http any any <> any any (content:"abc"; http_uri; gid:1; rev:"; http_uri; gid:1; rev:1;)
Actions #3

Updated by Victor Julien over 8 years ago

  • Subject changed from rule parsing issues to rule parsing issues: rev
alert http any any -> any any (content:"abc"; http_uri; rev:1; rev:";)

This is all for now, will start another run with different input to see if it finds more. Seems that all the above ones are related to problems with parsing the 'rev' keyword.

Actions #4

Updated by Andreas Herz over 8 years ago

I can't reproduce a "crash". I just end up with the rule not loaded:

[23788] 5/1/2016 -- 21:26:50 - (detect-rev.c:64) <Error> (DetectRevSetup) -- [ERRCODE: SC_ERR_INVALID_SIGNATURE(39)] - invalid character as arg to rev keyword
[23788] 5/1/2016 -- 21:26:50 - (detect.c:370) <Error> (DetectLoadSigFile) -- [ERRCODE: SC_ERR_INVALID_SIGNATURE(39)] - error parsing signature "alert http any any -> any any (content:"abc"; http_uri; rev:1; rev:";)" from file /home/andi/projects/suricata/rules/test.rules at line 1
[23788] 5/1/2016 -- 21:26:50 - (detect.c:525) <Info> (SigLoadSignatures) -- 1 rule files processed. 1 rules successfully loaded, 1 rules failed

I added a valid rule before and afterwards, they are loaded fine every time.

So what do you get and what should be the correct handling?

Actions #5

Updated by Andreas Herz over 8 years ago

Looks like some compiler option was missing/wrong, this is working and resulting in the error (thanks to Victor):

-O0 -ggdb -Werror -Wchar-subscripts -fno-strict-aliasing -fstack-protector-all -fsanitize=address -fno-omit-frame-pointer -march=native

Actions #6

Updated by Andreas Herz about 8 years ago

Suggestions from Victor and Jason (copied from Github, so they don't get lost):

I think Jason was suggesting we strip the quotes before the per keyword parse functions are called. That may be the best idea so far. That would have to be done in SigParseOptions(). I worry there might be unforeseen side effects though, but it's worth a try.
Actions #7

Updated by Victor Julien about 8 years ago

  • Status changed from Assigned to Closed
  • Target version changed from 70 to 3.0.1RC1
Actions

Also available in: Atom PDF