Skip to content

Comments

debug: fix list-x command line options with debug#8921

Closed
catenacyber wants to merge 1 commit intoOISF:masterfrom
catenacyber:debug-fuzz-list-6089-v2
Closed

debug: fix list-x command line options with debug#8921
catenacyber wants to merge 1 commit intoOISF:masterfrom
catenacyber:debug-fuzz-list-6089-v2

Conversation

@catenacyber
Copy link
Contributor

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/6089

Steps to reproduce :

  • build with ./src/tests/fuzz/oss-fuzz-configure.sh
  • run ./src/suricata --list-keywords or ./src/suricata --list-app-layer-protos

This fails with stack trace

Assertion failed: (!((g_engine_mode == ENGINE_MODE_UNKNOWN))), function EngineModeIsIPS, file suricata.c, line 224.
Abort trap: 6
    #2 0x10b15e236 in EngineModeIsIPS suricata.c:224
    #3 0x10b1a76c3 in ExceptionPolicyParse util-exception-policy.c:215
    #4 0x10ae3aff1 in AppLayerParserRegisterProtocolParsers app-layer-parser.c:1743
    #5 0x10ad79df8 in AppLayerSetup app-layer.c:973
    #6 0x10b1faffa in ListKeywords util-running-modes.c:37
    #7 0x10b16615c in SuricataMain suricata.c:2918

This hinders oss-fuzz which relies on these commands

Describe changes:

  • Adds a new engine mode to prevent the assertion to trigger

Another fix could just be to EngineModeSetIDS in ListKeywords

Modifies #8909 with ticket in message commit

Debug validation checks that engine is either IPS or IDS.
But listing keywords does not care.
So, adding a new mode for this case.

Ticket: OISF#6089
@codecov
Copy link

codecov bot commented May 25, 2023

Codecov Report

Merging #8921 (64f6243) into master (ebe0a7b) will increase coverage by 0.04%.
The diff coverage is 0.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8921      +/-   ##
==========================================
+ Coverage   82.30%   82.34%   +0.04%     
==========================================
  Files         969      969              
  Lines      273335   273339       +4     
==========================================
+ Hits       224961   225085     +124     
+ Misses      48374    48254     -120     
Flag Coverage Δ
fuzzcorpus 64.75% <0.00%> (+0.10%) ⬆️
suricata-verify 60.47% <0.00%> (+0.01%) ⬆️
unittests 62.95% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 14039

Copy link
Contributor

@lukashino lukashino left a comment

Choose a reason for hiding this comment

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

I think it would be better to go with EngineModeSetIDS approach so that we don't need to create a separate engine runmode. It is done so in other cases where Suricata doesn't particularly operate in IDS mode (only tests some part of Suricata).
Otherwise LGTM

@catenacyber
Copy link
Contributor Author

Replaced by #8956

@catenacyber catenacyber closed this Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants