Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect prefilter setupu8 4335 v6 #5937

Closed

Conversation

catenacyber
Copy link
Contributor

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/4112
https://redmine.openinfosecfoundation.org/issues/4335

Describe changes:

  • Adds error for impossible rules such as >255 for uint8 fields
  • converts icode to use the generic (many more keywords to do)
    • allows <> syntax for uint ranges to support existing syntax
    • corrects reverted ranges and issues a warning (so 20<>8 becomes 8<>20 ) to support existing syntax

Modifies #5923 by formatting and rewording commit

@codecov
Copy link

codecov bot commented Feb 26, 2021

Codecov Report

Merging #5937 (350cc10) into master (d708744) will increase coverage by 0.01%.
The diff coverage is 92.36%.

@@            Coverage Diff             @@
##           master    #5937      +/-   ##
==========================================
+ Coverage   76.67%   76.68%   +0.01%     
==========================================
  Files         604      604              
  Lines      187828   187821       -7     
==========================================
+ Hits       144008   144036      +28     
+ Misses      43820    43785      -35     
Flag Coverage Δ
fuzzcorpus 52.54% <90.21%> (-0.01%) ⬇️
suricata-verify 49.88% <26.59%> (+0.02%) ⬆️
unittests 63.07% <64.80%> (+<0.01%) ⬆️

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

@@ -373,6 +454,10 @@ DetectU8Data *DetectU8Parse (const char *u8str)
return NULL;
}
}
if (DetectU8Validate(&u8da)) {
SCLogError(SC_ERR_INVALID_VALUE, "Impossible value for uint8 condition");
Copy link
Member

Choose a reason for hiding this comment

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

can we provide a more helpful error here? Something that includes the keyword name and perhaps also the original value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok for the original value.
For the keyword name, should it be the caller of DetectU32Parse (such as DetectHTTP2windowSetup) which should print another message or should it transmit the keyword name to DetectU32Parse just for this debugging purpose ?

@catenacyber catenacyber added the needs rebase Needs rebase to master label Mar 10, 2021
@catenacyber
Copy link
Contributor Author

Replaced by #5966

lukashino pushed a commit to lukashino/suricata that referenced this pull request Mar 23, 2023
lukashino pushed a commit to lukashino/suricata that referenced this pull request Mar 23, 2023
lukashino pushed a commit to lukashino/suricata that referenced this pull request Mar 23, 2023
lukashino pushed a commit to lukashino/suricata that referenced this pull request Apr 4, 2023
lukashino pushed a commit to lukashino/suricata that referenced this pull request Apr 5, 2023
lukashino pushed a commit to lukashino/suricata that referenced this pull request Apr 18, 2023
lukashino pushed a commit to lukashino/suricata that referenced this pull request Apr 18, 2023
lukashino pushed a commit to lukashino/suricata that referenced this pull request Apr 24, 2023
lukashino pushed a commit to lukashino/suricata that referenced this pull request May 4, 2023
lukashino pushed a commit to lukashino/suricata that referenced this pull request May 4, 2023
lukashino pushed a commit to lukashino/suricata that referenced this pull request May 10, 2023
lukashino pushed a commit to lukashino/suricata that referenced this pull request May 10, 2023
victorjulien pushed a commit to victorjulien/suricata that referenced this pull request May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase Needs rebase to master
2 participants