new plugin for Azure NetApp Pools & Pool volumes#6171
Conversation
|
|
||
| return if (defined($self->{option_results}->{command_options}) && $self->{option_results}->{command_options} ne ''); | ||
|
|
||
| my $cmd_options = "monitor metrics list-definitions --resource '$options{resource}' --only-show-errors --output json"; |
There was a problem hiding this comment.
azure_list_resource_metrics_set_cmd builds a shell command by concatenating --resource and other option values into cmd_options; this allows command injection. Use safe argument passing or proper escaping.
Details
✨ AI Reasoning
A command string is being assembled by concatenating user-controllable option values directly into a shell-invoked command. This allows arbitrary shell metacharacters or injection to be included by an attacker, leading to command injection when executed. The problematic change constructs a command line with resource, resource_group and other fields interpolated without escaping or using a safe exec-array API.
🔧 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
|
|
||
| return if (defined($self->{option_results}->{command_options}) && $self->{option_results}->{command_options} ne ''); | ||
|
|
||
| my $cmd_options = "netappfiles volume list --account-name '$options{account_name}' --pool-name '$options{pool_name}' --resource-group '$options{resource_group}'"; |
There was a problem hiding this comment.
azure_list_netapp_volumes_set_cmd builds a shell command by concatenating --account-name, --pool-name and --resource-group option values into cmd_options; this allows command injection. Use safe argument passing or proper escaping.
Details
✨ AI Reasoning
A command string is being assembled by concatenating user-controllable option values directly into a shell-invoked command. This allows arbitrary shell metacharacters or injection to be included by an attacker, leading to command injection when executed. The problematic change constructs a command line with account_name, pool_name and resource_group interpolated without escaping or using a safe exec-array API.
🔧 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
| sub azure_list_netapp_volumes_set_url { | ||
| my ($self, %options) = @_; | ||
|
|
||
| my $url = $self->{management_endpoint} . "/subscriptions/" . $self->{subscription} . "/resourcegroups/" . |
There was a problem hiding this comment.
azure_list_netapp_volumes_set_url concatenates resource_group, account_name and pool_name directly into the URL path without encoding or validation; this may allow crafted input to manipulate the request URL.
Details
✨ AI Reasoning
The new API helper composes an HTTP URL by concatenating resource_group, account_name and pool_name directly into the path. These values originate from options and are included without escaping/encoding. This can cause malformed requests or allow an attacker to control request paths.
🔧 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
| return $cmd_options; | ||
| } | ||
|
|
||
| sub azure_list_netapp_volumes_metrics { |
There was a problem hiding this comment.
Method name mismatch: azcli defines azure_list_netapp_volumes_metrics, but callers use azure_list_netapp_volumes. This causes runtime failure with --custommode=azcli.
| sub azure_list_netapp_volumes_metrics { | |
| sub azure_list_netapp_volumes { |
Details
✨ AI Reasoning
1) The new list-volumes mode asks the custom object for a method named azure_list_netapp_volumes.
2) The API custom implementation provides that method, but the CLI custom implementation introduces a differently named method instead.
3) With the CLI custom mode selected, that call cannot be resolved, so execution fails before data retrieval.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
| sub azure_list_resource_metrics_set_url { | ||
| my ($self, %options) = @_; | ||
|
|
||
| my $url = $self->{management_endpoint}; |
There was a problem hiding this comment.
azure_list_resource_metrics_set_url concatenates $options{resource} directly into the request URL without encoding; untrusted resource values can manipulate the constructed HTTP path.
Details
✨ AI Reasoning
The management API URL is constructed by appending '/' . $options{resource} into the base management endpoint string. If $options{resource} is supplied externally, it may contain unexpected characters or sequences that change the resulting HTTP request (path traversal, injection into the URL). No encoding or validation is applied before concatenation.
🔧 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
| defined($metric_results{$resource_name}->{$metric_label_name}->{lc($aggregation)}) ? | ||
| $metric_results{$resource_name}->{$metric_label_name}->{lc($aggregation)} : |
There was a problem hiding this comment.
Metric value lookup uses $metric_label_name instead of $metric_name after checking $metric_name. This mismatched key path drops real data and stores 0 for metrics.
| defined($metric_results{$resource_name}->{$metric_label_name}->{lc($aggregation)}) ? | |
| $metric_results{$resource_name}->{$metric_label_name}->{lc($aggregation)} : | |
| defined($metric_results{$resource_name}->{$metric_name}->{lc($aggregation)}) ? | |
| $metric_results{$resource_name}->{$metric_name}->{lc($aggregation)} : |
Details
✨ AI Reasoning
1) Metric retrieval stores results by normalized metric name.
2) The selection loop correctly checks availability using that normalized key.
3) The final assignment then reads from a different key derived from the display label.
4) This contradiction makes collected values effectively unreachable and produces incorrect zero values.
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
NetApps documentation
Modes Available:
Capacity
Capacity metrics documentation
Volumes
Volume metrics documentation
I have implemented a process where the available metrics for the resource are first queried via the /providers/microsoft.insights/metricDefinitions endpoint.
for API
https://learn.microsoft.com/en-us/rest/api/monitor/metric-definitions/list?view=rest-monitor-2023-10-01&tabs=HTTP
for CLI
https://learn.microsoft.com/en-us/cli/azure/monitor/metrics?view=azure-cli-latest#az-monitor-metrics-list-definitions
This also allows you to determine the primaryAggregationType and use it if it has not been overridden with --aggregation.
Not all metrics are always available for all regions or features. That is why this is a good dynamic solution.
Fixes # (issue)
If you are fixing a github Issue already existing, mention it here.
Type of change
How this pull request can be tested ?
pool-capacity.debug.txt
volumes.debug.txt
Checklist
Summary by Aikido
🚀 New Features
⚡ Enhancements
More info