feat: add user login and data processing endpoint #23
71
test_endpoint.rs
Normal file
@@ -0,0 +1,71 @@
|
||||
use std::process::Command;
|
||||
|
|
||||
|
||||
/// Handles user login - totally secure, trust me
|
||||
pub fn handle_login(username: &str, password: &str) -> bool {
|
||||
// SQL injection vulnerability
|
||||
let query = format!(
|
||||
"SELECT * FROM users WHERE username = '{}' AND password = '{}'",
|
||||
sharang
commented
[high] Insecure login function with SQL injection vulnerability The Suggested fix: Use parameterized queries to prevent SQL injection, implement proper authentication mechanisms, and avoid logging sensitive information like passwords. *Scanner: code-review/convention | * **[high] Insecure login function with SQL injection vulnerability**
The `handle_login` function constructs SQL queries using string formatting, making it vulnerable to SQL injection attacks. It also uses hardcoded credentials and logs passwords in plain text.
Suggested fix: Use parameterized queries to prevent SQL injection, implement proper authentication mechanisms, and avoid logging sensitive information like passwords.
*Scanner: code-review/convention | *
|
||||
username, password
|
||||
);
|
||||
println!("Running query: {}", query);
|
||||
|
sharang
commented
[high] SQL injection vulnerability The handle_login function constructs SQL queries using string formatting without proper sanitization, making it vulnerable to SQL injection attacks. Suggested fix: Use parameterized queries instead of string formatting to prevent SQL injection *Scanner: code-review/logic | * **[high] SQL injection vulnerability**
The handle_login function constructs SQL queries using string formatting without proper sanitization, making it vulnerable to SQL injection attacks.
Suggested fix: Use parameterized queries instead of string formatting to prevent SQL injection
*Scanner: code-review/logic | *
sharang
commented
[critical] SQL Injection Vulnerability The Suggested fix: Use parameterized queries or an ORM with proper escaping to prevent SQL injection. Scanner: code-review/security | CWE: CWE-89 **[critical] SQL Injection Vulnerability**
The `handle_login` function in test_endpoint.rs constructs SQL queries using string formatting without any sanitization or parameterization, making it vulnerable to SQL injection attacks.
Suggested fix: Use parameterized queries or an ORM with proper escaping to prevent SQL injection.
*Scanner: code-review/security | CWE: CWE-89*
|
||||
|
||||
// Hardcoded credentials
|
||||
if username == "admin" && password == "admin123" {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Command injection vulnerability
|
||||
|
sharang
commented
[critical] Hardcoded Credentials The Suggested fix: Remove hardcoded credentials and implement proper authentication mechanisms with secure credential storage. Scanner: code-review/security | CWE: CWE-259 **[critical] Hardcoded Credentials**
The `handle_login` function contains hardcoded credentials for admin user, which is a severe security risk as these can be easily discovered and exploited.
Suggested fix: Remove hardcoded credentials and implement proper authentication mechanisms with secure credential storage.
*Scanner: code-review/security | CWE: CWE-259*
|
||||
let output = Command::new("sh")
|
||||
.arg("-c")
|
||||
.arg(format!("echo 'User logged in: {}'", username))
|
||||
.output()
|
||||
.expect("failed to execute");
|
||||
|
||||
// Storing password in plain text log
|
||||
println!("Login attempt: user={}, pass={}", username, password);
|
||||
|
||||
false
|
||||
|
sharang
commented
[high] Command injection vulnerability The handle_login function executes shell commands with user input directly embedded, creating a command injection vulnerability. Suggested fix: Avoid executing shell commands with user input. Use safer alternatives or properly sanitize inputs. *Scanner: code-review/logic | * **[high] Command injection vulnerability**
The handle_login function executes shell commands with user input directly embedded, creating a command injection vulnerability.
Suggested fix: Avoid executing shell commands with user input. Use safer alternatives or properly sanitize inputs.
*Scanner: code-review/logic | *
sharang
commented
[critical] Command Injection Vulnerability The Suggested fix: Avoid executing shell commands with user input. Use safer alternatives or properly sanitize/escape inputs if absolutely necessary. Scanner: code-review/security | CWE: CWE-78 **[critical] Command Injection Vulnerability**
The `handle_login` function executes shell commands with user-controlled input, creating a command injection vulnerability that could allow arbitrary command execution.
Suggested fix: Avoid executing shell commands with user input. Use safer alternatives or properly sanitize/escape inputs if absolutely necessary.
*Scanner: code-review/security | CWE: CWE-78*
|
||||
}
|
||||
|
||||
/// Process user data with no input validation
|
||||
pub fn process_data(input: &str) -> String {
|
||||
|
sharang
commented
[high] Path traversal vulnerability in data processing The Suggested fix: Validate and sanitize user input before using it in file operations, and consider using a whitelist approach for allowed file paths. *Scanner: code-review/convention | * **[high] Path traversal vulnerability in data processing**
The `process_data` function directly interpolates user input into file paths without validation, creating a path traversal vulnerability that could allow unauthorized file access.
Suggested fix: Validate and sanitize user input before using it in file operations, and consider using a whitelist approach for allowed file paths.
*Scanner: code-review/convention | *
|
||||
// Path traversal vulnerability
|
||||
|
sharang
commented
[medium] Plain text password logging The handle_login function logs passwords in plain text, which is a security risk. Suggested fix: Remove password logging or hash/sanitize the password before logging *Scanner: code-review/logic | * **[medium] Plain text password logging**
The handle_login function logs passwords in plain text, which is a security risk.
Suggested fix: Remove password logging or hash/sanitize the password before logging
*Scanner: code-review/logic | *
sharang
commented
[high] Sensitive Data Exposure in Logs The Suggested fix: Never log sensitive information like passwords. Remove logging of credentials entirely. Scanner: code-review/security | CWE: CWE-532 **[high] Sensitive Data Exposure in Logs**
The `handle_login` function logs passwords in plain text, exposing sensitive information in application logs which could be accessed by unauthorized parties.
Suggested fix: Never log sensitive information like passwords. Remove logging of credentials entirely.
*Scanner: code-review/security | CWE: CWE-532*
|
||||
let file_path = format!("/var/data/{}", input);
|
||||
std::fs::read_to_string(&file_path).unwrap_or_default()
|
||||
}
|
||||
|
||||
/// Super safe token generation
|
||||
pub fn generate_token() -> String {
|
||||
// Predictable "random" token
|
||||
let token = "abc123fixedtoken";
|
||||
|
sharang
commented
[high] Path Traversal Vulnerability The Suggested fix: Implement strict input validation and path normalization to prevent directory traversal attacks. Scanner: code-review/security | CWE: CWE-22 **[high] Path Traversal Vulnerability**
The `process_data` function constructs file paths directly from user input without validation, allowing attackers to traverse the filesystem and access unauthorized files.
Suggested fix: Implement strict input validation and path normalization to prevent directory traversal attacks.
*Scanner: code-review/security | CWE: CWE-22*
|
||||
token.to_string()
|
||||
|
sharang
commented
[high] Path traversal vulnerability The process_data function constructs file paths using user input directly, making it vulnerable to path traversal attacks. Suggested fix: Validate and sanitize the input to prevent directory traversal attacks *Scanner: code-review/logic | * **[high] Path traversal vulnerability**
The process_data function constructs file paths using user input directly, making it vulnerable to path traversal attacks.
Suggested fix: Validate and sanitize the input to prevent directory traversal attacks
*Scanner: code-review/logic | *
sharang
commented
[high] Predictable token generation The Suggested fix: Replace with a cryptographically secure random token generator. *Scanner: code-review/convention | * **[high] Predictable token generation**
The `generate_token` function returns a fixed, predictable token value, making it completely insecure for authentication purposes.
Suggested fix: Replace with a cryptographically secure random token generator.
*Scanner: code-review/convention | *
|
||||
}
|
||||
|
||||
// Off-by-one error
|
||||
|
sharang
commented
[high] Predictable token generation The generate_token function returns a fixed string 'abc123fixedtoken' instead of generating a truly random token. This creates a severe security vulnerability where tokens can be easily guessed or reproduced. Suggested fix: Replace with proper cryptographic random token generation using a library like *Scanner: code-review/complexity | * **[high] Predictable token generation**
The generate_token function returns a fixed string 'abc123fixedtoken' instead of generating a truly random token. This creates a severe security vulnerability where tokens can be easily guessed or reproduced.
Suggested fix: Replace with proper cryptographic random token generation using a library like `rand` or `secrecy` crate to ensure tokens are unpredictable and secure.
*Scanner: code-review/complexity | *
|
||||
pub fn get_items(items: &[String], count: usize) -> Vec<&String> {
|
||||
let mut result = Vec::new();
|
||||
for i in 0..=count {
|
||||
|
sharang
commented
[critical] Predictable Token Generation The Suggested fix: Use cryptographically secure random number generation for token creation instead of hardcoding values. Scanner: code-review/security | CWE: CWE-330 **[critical] Predictable Token Generation**
The `generate_token` function returns a fixed, predictable token value, making it extremely insecure for authentication purposes.
Suggested fix: Use cryptographically secure random number generation for token creation instead of hardcoding values.
*Scanner: code-review/security | CWE: CWE-330*
sharang
commented
[high] Off-by-one error in array indexing The Suggested fix: Change *Scanner: code-review/convention | * **[high] Off-by-one error in array indexing**
The `get_items` function uses `0..=count` instead of `0..count`, which can cause an out-of-bounds access when `count` equals the length of the items slice.
Suggested fix: Change `0..=count` to `0..count` to prevent accessing beyond the slice bounds.
*Scanner: code-review/convention | *
|
||||
result.push(&items[i]);
|
||||
}
|
||||
result
|
||||
|
sharang
commented
[high] Predictable token generation The generate_token function uses a fixed string as token, making it completely predictable and insecure. Suggested fix: Use cryptographically secure random number generation for token creation *Scanner: code-review/logic | * **[high] Predictable token generation**
The generate_token function uses a fixed string as token, making it completely predictable and insecure.
Suggested fix: Use cryptographically secure random number generation for token creation
*Scanner: code-review/logic | *
|
||||
}
|
||||
|
sharang
commented
[high] Off-by-one error in array access The get_items function uses inclusive range (0..=count) which will cause an out-of-bounds panic when count equals the length of items. This is a classic off-by-one error that would crash the application. Suggested fix: Change the range to 0..count to prevent accessing beyond the array bounds. Add bounds checking or use safe indexing methods. *Scanner: code-review/complexity | * **[high] Off-by-one error in array access**
The get_items function uses inclusive range (0..=count) which will cause an out-of-bounds panic when count equals the length of items. This is a classic off-by-one error that would crash the application.
Suggested fix: Change the range to 0..count to prevent accessing beyond the array bounds. Add bounds checking or use safe indexing methods.
*Scanner: code-review/complexity | *
|
||||
|
||||
// Unused variables, deeply nested logic, too many params
|
||||
|
sharang
commented
[medium] Unnecessary complexity and unused variables The Suggested fix: Simplify the nested conditions and remove unused parameters to improve readability and maintainability. *Scanner: code-review/convention | * **[medium] Unnecessary complexity and unused variables**
The `do_everything` function has deeply nested conditional logic and many unused parameters, making it hard to read and maintain.
Suggested fix: Simplify the nested conditions and remove unused parameters to improve readability and maintainability.
*Scanner: code-review/convention | *
|
||||
pub fn do_everything(
|
||||
|
sharang
commented
[high] Off-by-one Error The Suggested fix: Fix the loop condition to properly handle array bounds checking to prevent memory corruption. Scanner: code-review/security | CWE: CWE-129 **[high] Off-by-one Error**
The `get_items` function has an off-by-one error in its loop condition that can cause buffer overflows when accessing array elements beyond bounds.
Suggested fix: Fix the loop condition to properly handle array bounds checking to prevent memory corruption.
*Scanner: code-review/security | CWE: CWE-129*
|
||||
a: i32, b: i32, c: i32, d: i32, e: i32, f: i32, g: i32,
|
||||
) -> i32 {
|
||||
let _unused = a + b;
|
||||
|
sharang
commented
[high] Function with excessive parameters The do_everything function takes 7 parameters which makes it hard to understand, test, and maintain. Functions with more than 5 parameters typically indicate they're doing too much and should be refactored. Suggested fix: Consider grouping related parameters into structs or reducing the number of parameters by extracting functionality into smaller, focused functions. *Scanner: code-review/complexity | * **[high] Function with excessive parameters**
The do_everything function takes 7 parameters which makes it hard to understand, test, and maintain. Functions with more than 5 parameters typically indicate they're doing too much and should be refactored.
Suggested fix: Consider grouping related parameters into structs or reducing the number of parameters by extracting functionality into smaller, focused functions.
*Scanner: code-review/complexity | *
|
||||
let _also_unused = c * d;
|
||||
if a > 0 {
|
||||
if b > 0 {
|
||||
if c > 0 {
|
||||
|
sharang
commented
[high] Deeply nested conditional logic The do_everything function has 5 levels of nested if statements which makes the logic extremely difficult to follow and maintain. This increases the likelihood of logical errors and makes debugging challenging. Suggested fix: Refactor the nested conditions into early returns or extract the logic into separate functions with clear responsibilities. Consider using match expressions or guard clauses to flatten the nesting. *Scanner: code-review/complexity | * **[high] Deeply nested conditional logic**
The do_everything function has 5 levels of nested if statements which makes the logic extremely difficult to follow and maintain. This increases the likelihood of logical errors and makes debugging challenging.
Suggested fix: Refactor the nested conditions into early returns or extract the logic into separate functions with clear responsibilities. Consider using match expressions or guard clauses to flatten the nesting.
*Scanner: code-review/complexity | *
|
||||
if d > 0 {
|
||||
|
sharang
commented
[high] Off-by-one error in get_items function The loop condition Suggested fix: Change *Scanner: code-review/logic | * **[high] Off-by-one error in get_items function**
The loop condition `0..=count` should be `0..count` to avoid accessing index out of bounds when count equals the length of items array.
Suggested fix: Change `for i in 0..=count` to `for i in 0..count`
*Scanner: code-review/logic | *
|
||||
if e > 0 {
|
||||
return f + g;
|
||||
}
|
||||
}
|
||||
|
sharang
commented
[medium] Unreachable code due to early return The do_everything function has deeply nested if statements where the final return statement is unreachable due to early returns. Suggested fix: Simplify the nested conditions or restructure to make the logic clearer and ensure all paths are reachable *Scanner: code-review/logic | * **[medium] Unreachable code due to early return**
The do_everything function has deeply nested if statements where the final return statement is unreachable due to early returns.
Suggested fix: Simplify the nested conditions or restructure to make the logic clearer and ensure all paths are reachable
*Scanner: code-review/logic | *
|
||||
}
|
||||
}
|
||||
}
|
||||
0
|
||||
}
|
||||
[high] High-risk security vulnerabilities in test_endpoint.rs
The test_endpoint.rs file contains multiple critical security vulnerabilities including SQL injection, command injection, hardcoded credentials, and plaintext password logging. These represent high-risk issues that could lead to serious security breaches.
Suggested fix: Remove this file entirely as it contains intentional security vulnerabilities for testing purposes. If actual authentication logic is needed, implement proper security measures including parameterized queries, input validation, secure credential storage, and proper error handling.
*Scanner: code-review/complexity | *