perl_lsp_diagnostics/lints/
security.rs

1//! Security-focused lint checks
2//!
3//! This module provides lint checks that detect common security anti-patterns
4//! in Perl code. These are patterns that `perl -c` and PPI cannot catch because
5//! they require AST-level analysis.
6//!
7//! # Diagnostic codes
8//!
9//! | Code | Severity | Description |
10//! |------|----------|-------------|
11//! | `security-two-arg-open` | Warning | `open(FH, ">file")` -- use 3-arg open for safety |
12//! | `security-string-eval` | Warning | `eval "$string"` -- string eval is a security risk |
13//! | `security-backtick-exec` | Information | Backtick/qx command execution detected |
14
15use perl_diagnostics_codes::DiagnosticCode;
16use perl_parser_core::ast::{Node, NodeKind};
17
18use super::super::walker::walk_node;
19use perl_lsp_diagnostic_types::{Diagnostic, DiagnosticSeverity, RelatedInformation};
20
21/// Check for security anti-patterns
22///
23/// This function walks the AST looking for:
24/// - Two-argument `open` calls (should use 3-arg form)
25/// - String `eval` (security risk vs. block `eval`)
26/// - Backtick/qx command execution (ensure input is sanitized)
27pub fn check_security(node: &Node, diagnostics: &mut Vec<Diagnostic>) {
28    walk_node(node, &mut |n| {
29        match &n.kind {
30            NodeKind::FunctionCall { name, args } => {
31                check_two_arg_open(name, args, n, diagnostics);
32                check_string_eval(name, args, n, diagnostics);
33            }
34
35            // The parser produces Eval { block } for both `eval { ... }` and
36            // `eval "string"`.  Block evals are safe (exception handling);
37            // string/variable evals are a security risk.
38            NodeKind::Eval { block } => {
39                check_eval_node(block, n, diagnostics);
40            }
41
42            // Backtick strings: the parser stores `cmd` and qx(cmd) as
43            // String { value: "`cmd`", interpolated: true }
44            NodeKind::String { value, interpolated: true } if is_backtick_string(value) => {
45                diagnostics.push(Diagnostic {
46                    range: (n.location.start, n.location.end),
47                    severity: DiagnosticSeverity::Information,
48                    code: Some(DiagnosticCode::SecurityBacktickExec.as_str().to_string()),
49                    message: "Command execution detected. Ensure input is sanitized.".to_string(),
50                    related_information: vec![RelatedInformation {
51                        location: (n.location.start, n.location.end),
52                        message: "Consider using open() with a pipe, or IPC::Run for safer command execution with proper input validation".to_string(),
53                    }],
54                    tags: Vec::new(),
55                    suggestion: Some(
56                        "Use open(my $fh, '-|', @cmd) or IPC::Run for safer command execution"
57                            .to_string(),
58                    ),
59                });
60            }
61
62            _ => {}
63        }
64    });
65}
66
67/// Detect string `eval` from `NodeKind::Eval` nodes.
68///
69/// The parser produces `Eval { block }` for both `eval { ... }` and
70/// `eval "string"`. Block evals (`eval { ... }`) are safe exception handling;
71/// string/variable evals are a security risk.
72fn check_eval_node(block: &Node, eval_node: &Node, diagnostics: &mut Vec<Diagnostic>) {
73    let is_string_eval = matches!(&block.kind, NodeKind::String { .. } | NodeKind::Variable { .. })
74        || matches!(&block.kind, NodeKind::Binary { op, .. } if op == ".");
75
76    if !is_string_eval {
77        return;
78    }
79
80    diagnostics.push(Diagnostic {
81        range: (eval_node.location.start, eval_node.location.end),
82        severity: DiagnosticSeverity::Warning,
83        code: Some(DiagnosticCode::SecurityStringEval.as_str().to_string()),
84        message: "String eval is a security risk. Consider eval { } for exception handling."
85            .to_string(),
86        related_information: vec![RelatedInformation {
87            location: (eval_node.location.start, eval_node.location.end),
88            message: "String eval executes arbitrary Perl code at runtime. If the string contains user input, this allows code injection.".to_string(),
89        }],
90        tags: Vec::new(),
91        suggestion: Some(
92            "Use eval { } for exception handling, or consider safer alternatives like Try::Tiny"
93                .to_string(),
94        ),
95    });
96}
97
98/// Detect two-argument `open` calls.
99///
100/// `open(FH, ">file")` is unsafe because the mode and filename are combined,
101/// allowing shell injection if the filename comes from user input.
102fn check_two_arg_open(name: &str, args: &[Node], node: &Node, diagnostics: &mut Vec<Diagnostic>) {
103    if name != "open" || args.len() != 2 {
104        return;
105    }
106
107    diagnostics.push(Diagnostic {
108        range: (node.location.start, node.location.end),
109        severity: DiagnosticSeverity::Warning,
110        code: Some(DiagnosticCode::TwoArgOpen.as_str().to_string()),
111        message: "Use 3-argument open for safety: open(my $fh, '>', 'file')".to_string(),
112        related_information: vec![RelatedInformation {
113            location: (node.location.start, node.location.end),
114            message: "Two-argument open combines mode and filename, which can allow shell injection if the filename is derived from user input".to_string(),
115        }],
116        tags: Vec::new(),
117        suggestion: Some("Replace with 3-arg form: open(my $fh, '>', $file)".to_string()),
118    });
119}
120
121/// Detect string `eval` calls.
122///
123/// `eval "code"` executes arbitrary Perl code at runtime, which is a security
124/// risk when the string contains user input. Block eval (`eval { ... }`) is
125/// safe and used for exception handling.
126fn check_string_eval(name: &str, args: &[Node], node: &Node, diagnostics: &mut Vec<Diagnostic>) {
127    if name != "eval" {
128        return;
129    }
130
131    // Check that the first argument is a string (not a block/other expression).
132    // eval { ... } is parsed as NodeKind::Eval, not FunctionCall, so reaching
133    // here already means this is the function-call form. But we still check
134    // the arg is a string to avoid false positives on eval($coderef).
135    let is_string_arg = args.first().is_some_and(|arg| match &arg.kind {
136        NodeKind::String { .. } | NodeKind::Variable { .. } => true,
137        NodeKind::Binary { op, .. } if op == "." => true,
138        _ => false,
139    });
140
141    if !is_string_arg && !args.is_empty() {
142        return;
143    }
144
145    diagnostics.push(Diagnostic {
146        range: (node.location.start, node.location.end),
147        severity: DiagnosticSeverity::Warning,
148        code: Some(DiagnosticCode::SecurityStringEval.as_str().to_string()),
149        message: "String eval is a security risk. Consider eval { } for exception handling."
150            .to_string(),
151        related_information: vec![RelatedInformation {
152            location: (node.location.start, node.location.end),
153            message: "String eval executes arbitrary Perl code at runtime. If the string contains user input, this allows code injection.".to_string(),
154        }],
155        tags: Vec::new(),
156        suggestion: Some(
157            "Use eval { } for exception handling, or consider safer alternatives like Try::Tiny"
158                .to_string(),
159        ),
160    });
161}
162
163/// Check if a string value represents a backtick command execution.
164///
165/// The parser stores backtick literals (`` `cmd` ``) and qx(cmd) as
166/// `String { value: "`cmd`", interpolated: true }`.
167fn is_backtick_string(value: &str) -> bool {
168    value.starts_with('`') && value.ends_with('`') && value.len() >= 2
169}
perl_lsp_diagnostics/lints/security.rs

perl_lsp_diagnostics/lints/
security.rs
