improve APC UPS inputlines#6147
Conversation
| instance => $instance | ||
| ); | ||
|
|
||
| next if is_excluded( |
There was a problem hiding this comment.
User-controlled include/exclude patterns (include_input_type/exclude_input_type) are passed directly to is_excluded. This can allow regex injection / ReDoS via crafted option values. Validate or safely escape/limit regex input before use.
Details
✨ AI Reasoning
The code now calls is_excluded(...) with patterns coming directly from option_results (user input). These values are likely used as regular expressions or evaluated as patterns inside is_excluded. Supplying untrusted patterns to regex-matching utilities can allow regex injection or ReDoS (catastrophic backtracking) attacks or unintended behavior. The new calls were added in this PR and therefore represent an introduced risk.
🔧 How do I fix it?
Use parameterized queries with placeholders, array-based command execution (no shell interpretation), or properly escaped arguments using vetted libraries. Avoid dynamic queries/commands built with user input concatenation.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
| my $instance = $phase_result->{inputNumber} . $phase_result->{inputPhaseIndex}; | ||
|
|
||
| next if !defined($self->{inputs}->{$input_num});# input has just be excluded on inputs | ||
| next if is_excluded( |
There was a problem hiding this comment.
User-controlled include/exclude patterns (include_input_phase/exclude_input_phase) are passed directly to is_excluded. This can allow regex injection / ReDoS via crafted option values. Validate or safely escape/limit regex input before use.
Details
✨ AI Reasoning
The diff adds another is_excluded(...) call that uses option_results include_input_phase/exclude_input_phase directly. As above, these options are user-controlled and likely interpreted as regex patterns by is_excluded, enabling regex injection or ReDoS if maliciously crafted. This call was introduced in the PR and therefore is a new risk.
🔧 How do I fix it?
Use parameterized queries with placeholders, array-based command execution (no shell interpretation), or properly escaped arguments using vetted libraries. Avoid dynamic queries/commands built with user input concatenation.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
Community contributors
Description
The first problem is that there are APCs that do not provide input line voltage information. Since this SNMP query is set to
nothing_quit => 1, we do not receive any information about the input phases, even though it is actually available.The second problem is that there are, for example, the Galaxy series, which not only have the L-N but also the L-L
These are two different types of voltages:
• Phase to Neutral (L–N) → ~230 V
• Phase to Phase (L–L) → ~400 V
So you have:
• 3× L–N voltages (L1, L2, L3 → each about 230 V)
• 3× L–L voltages (L1–L2, L2–L3, L3–L1 → each about 400 V)
In the SNMP table
The index is one number longer as the normal one
so what I changed in the plugin?
• I've updated the (mode) class to meet the new standard. That is, counter constants, is_excluded, and include-exclude filters.
• This Galaxy doesn't have any InputLine data, so just the phases and the frequency. That's why I removed the “nothing_quit => 1”. It's not a problem because the code still runs afterward.
• This OID regex match I extend to possible 18 long OID
• The input number and the inputPhaseIndex I get from the SNMP table. Working for all APCs
• Filter by input_type (main, bypass)
• Filter by input phase
• help
So with the default exclude we exclude all L-L (3 numbers long instances).
without filter
using exclude filter
So you can see the plugin with this default should be work still in the same way as before but is working fine also for this “big” Galaxy” 3 Phase APCs.
Type of change
How this pull request can be tested ?
i will send a download link via private mail with 4 snmpwalks
Checklist
Centreon team (internal PR)
Description
PLEASE MAKE SURE THAT THE BRANCH PR INCLUDES JIRA TICKET ID
Please include a short resume of the changes and what is the purpose of this pull request.
Any relevant information should be added to help reviewers to understand what are the stakes
of the pull request.
Fixes # (issue)
If you are fixing a github Issue already existing, mention it here.
If you are fixing one or more JIRA ticket, mention it here too.
Type of change
How this pull request can be tested ?
Please describe the procedure to verify that the goal of the PR is matched.
Provide clear instructions so that it can be correctly tested.
Mention the automated tests included in this FOR (what they test like mode/option combinations).
Checklist
Summary by Aikido
⚡ Enhancements
🔧 Refactors
📚 Documentation
More info