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

fix: align TIME_MON variable's behavior #3306

Open
wants to merge 1 commit into
base: v3/master
Choose a base branch
from

Conversation

airween
Copy link
Member

@airween airween commented Nov 22, 2024

what

This PR changes TIME_MON variable's behavior and fixes #3305.

Also fixes the regex in changed variable's test file (the old one allows any number, eg. 0 or 1111 which makes no sense; the new one allows between 1 and 12.).

why

As the issue describes in libmodsecurity3 the TIME_MON variable can be between 0 and 11. mod_security2 uses values between 1 and 12 - which is much more natural.

references

See issue #3305.

@airween
Copy link
Member Author

airween commented Nov 22, 2024

@M4tteoP could you review this PR? Thanks!

Copy link

sonarcloud bot commented Nov 22, 2024

@airween
Copy link
Member Author

airween commented Nov 22, 2024

There is a very interesting check result:

  • in first attempt the cppcheck step showed an error here
  • I re-run that check and then the error disappeared - see here

I tried this on my local environment (with a newer cppcheck (2.16.0)), and that also reports this issue:

$ cppcheck -U YYSTYPE -U MBEDTLS_MD5_ALT -U MBEDTLS_SHA1_ALT -D MS_CPPCHECK_DISABLED_FOR_PARSER -U YY_USER_INIT --suppressions-list=./test/cppcheck_suppressions.txt --inline-suppr --enable=warning,style,performance,portability,unusedFunction,missingInclude --inconclusive --template="warning: {file},{line},{severity},{id},{message}" -I headers -I . -I others -I src -I others/mbedtls/include --error-exitcode=1 -i "src/parser/seclang-parser.cc" -i "src/parser/seclang-scanner.cc" -i others --std=c++17 --force --verbose src/utils/system.cc 
Checking src/utils/system.cc ...
Defines:MS_CPPCHECK_DISABLED_FOR_PARSER=1
Undefines: MBEDTLS_MD5_ALT; MBEDTLS_SHA1_ALT; YYSTYPE; YY_USER_INIT
Includes: -Iheaders/ -I./ -Iothers/ -Isrc/ -Iothers/mbedtls/include/
Platform:native
Checking src/utils/system.cc: MS_CPPCHECK_DISABLED_FOR_PARSER=1...
Checking src/utils/system.cc: MS_CPPCHECK_DISABLED_FOR_PARSER=1;NO_LOGS...
Checking src/utils/system.cc: MS_CPPCHECK_DISABLED_FOR_PARSER=1;S_IFMT;S_IFREG;S_ISREG...
warning: src/utils/system.cc,213,error,syntaxError,syntax error
Checking src/utils/system.cc: MS_CPPCHECK_DISABLED_FOR_PARSER=1;WIN32...
Checking src/utils/system.cc: MS_CPPCHECK_DISABLED_FOR_PARSER=1;_MSC_VER...
Checking src/utils/system.cc: MS_CPPCHECK_DISABLED_FOR_PARSER=1;__GNUC__...
Checking src/utils/system.cc: MS_CPPCHECK_DISABLED_FOR_PARSER=1;__OpenBSD__...

@eduar-hte do you have any idea?

@M4tteoP
Copy link
Contributor

M4tteoP commented Nov 22, 2024

@M4tteoP could you review this PR? Thanks!

It looks good to me! I just don't know how this change will be handled regarding breaking changes. Rules relying on TIME_MON might require changes after this fix.

Copy link
Contributor

@fzipi fzipi left a comment

Choose a reason for hiding this comment

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

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TIME_MON variable: inconsistent ranges between 2.x and 3.x engines.
3 participants