Skip to content

chore: fix cppcheck warning #3434

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

Open
wants to merge 7 commits into
base: v3/master
Choose a base branch
from

Conversation

airween
Copy link
Member

@airween airween commented Aug 11, 2025

what

This PR collects a few small fixes which solves the issues with current (2.18.0) cppcheck.

Changes:

  • src/operators/fuzzy_hash.cc: variable iss was declared as std::istream and later it was converted unnecessary
  • src/operators/inspect_file.cc: same issue
  • src/operators/pm_from_file.cc: similar issue, but here there are two options based on resource path; if the resource begins with https:// then we should read from stream. Note that std::stringstream (case of http resource) and std::ifstream derived both from std::istream
  • src/operators/rbl.cc: changed implicit cast to reinterpret_cast<> and add error report (if the socket type is not AF_INET (IPv4))
  • src/operators/validate_dtd.cc: changed implicit cast to reinterpret_cast<>
  • src/variables/xml.cc: same solution
  • src/parser/seclang-parser.hh: this is a generated file by Bison so we can't change its content; therefore I added a new supression into test/cppcheck_suppressions.txt

why

There are a few warnings by cppcheck in recent PR's.

references

See PR #3432.

@airween airween requested a review from theseion August 11, 2025 16:03
@airween airween added the 3.x Related to ModSecurity version 3.x label Aug 11, 2025
Copy link
Collaborator

@theseion theseion left a comment

Choose a reason for hiding this comment

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

Can't say that I understand all the changes, but I didn't spot any obvious errors.

@airween
Copy link
Member Author

airween commented Aug 11, 2025

Can't say that I understand all the changes,

Below I attached the link of the failed action and a short description how to see them. I fixed all occurrences.

but I didn't spot any obvious errors.

Here is the output of cppcheck action:

https://github.com/owasp-modsecurity/ModSecurity/actions/runs/16874871780/job/47804383442?pr=3432

Unfortunately it's hard to see where are the failed checks, probably you can download raw logs, extract them and find the pattern warning in cppcheck's output - like this:

2025-08-11T10:24:18.9709660Z warning: src/operators/fuzzy_hash.cc,51,warning,dangerousTypeCast,Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast

Mostly those are type cast issues.

There are a few new SonarCloud errors here, I'm going to fix them before I merge this PR. I'll let you know if I'm done.

@airween
Copy link
Member Author

airween commented Aug 11, 2025

Here are the list of type cast issues:

2025-08-11T10:24:18.9709660Z warning: src/operators/fuzzy_hash.cc,51,warning,dangerousTypeCast,Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast
2025-08-11T10:24:55.8865180Z warning: src/operators/inspect_file.cc,41,warning,dangerousTypeCast,Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast
2025-08-11T10:26:20.1775780Z warning: src/operators/pm_from_file.cc,69,warning,dangerousTypeCast,Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast
2025-08-11T10:26:30.5678000Z warning: src/operators/rbl.cc,230,warning,dangerousTypeCast,Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast
2025-08-11T10:27:42.0275870Z warning: src/operators/validate_dtd.cc,48,warning,dangerousTypeCast,Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast
2025-08-11T10:34:39.0309300Z warning: src/variables/xml.cc,82,warning,dangerousTypeCast,Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast
2025-08-11T10:34:39.0313990Z warning: src/variables/xml.cc,94,warning,dangerousTypeCast,Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast
2025-08-11T10:34:39.0314930Z warning: src/variables/xml.cc,95,warning,dangerousTypeCast,Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast
2025-08-11T10:34:39.0315820Z warning: src/variables/xml.cc,96,warning,dangerousTypeCast,Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast

plus the seclang-parser.hh issues, like this one:

2025-08-11T14:56:05.5233700Z warning: seclang-parser.hh,526,warning,uninitMemberVar,Member variable 'value_type::emplace < std :: string >' is not initialized in the constructor. Member variables of native types, pointers, or references are left uninitialized when the class is instantiated. That may cause bugs or undefined behavior.

Copy link

@airween
Copy link
Member Author

airween commented Aug 11, 2025

Okay, I think I'm done.

@airween airween requested a review from theseion August 11, 2025 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Related to ModSecurity version 3.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants