Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions vulnerable.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
#include <iostream>
#include <cstring>
#include <vector>
#include <string>
#include <cstdlib>
#include <ctime>

using namespace std;

// 1. 硬编码敏感信息 (CWE-798)
// SAST 会扫描变量名(如 api_key, secret)和高熵字符串
const string AWS_SECRET_KEY = "AKIAIOSFODNN7EXAMPLE";
Comment on lines +10 to +12

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Hardcoded AWS credentials detected.

Hardcoded secrets in source code are a severe security vulnerability. This credential will be:

  • Permanently visible in version control history
  • Exposed in compiled binaries
  • Accessible to anyone with repository access
🔎 Recommended secure alternatives
-// 1. 硬编码敏感信息 (CWE-798)
-// SAST 会扫描变量名(如 api_key, secret)和高熵字符串
-const string AWS_SECRET_KEY = "AKIAIOSFODNN7EXAMPLE"; 
+// Load secrets from environment variables or secure secret management system
+const char* aws_key = getenv("AWS_SECRET_KEY");
+if (!aws_key) {
+    cerr << "Error: AWS_SECRET_KEY environment variable not set" << endl;
+    exit(1);
+}
+const string AWS_SECRET_KEY = aws_key;

Alternatively, use a proper secret management solution like AWS Secrets Manager, HashiCorp Vault, or encrypted configuration files.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 1. 硬编码敏感信息 (CWE-798)
// SAST 会扫描变量名(如 api_key, secret)和高熵字符串
const string AWS_SECRET_KEY = "AKIAIOSFODNN7EXAMPLE";
// Load secrets from environment variables or secure secret management system
const char* aws_key = getenv("AWS_SECRET_KEY");
if (!aws_key) {
cerr << "Error: AWS_SECRET_KEY environment variable not set" << endl;
exit(1);
}
const string AWS_SECRET_KEY = aws_key;
🤖 Prompt for AI Agents
In vulnerable.cpp around lines 10 to 12, the code contains a hardcoded AWS
secret key which must be removed; replace the hardcoded value by retrieving
credentials from a secure source (e.g., environment variable, AWS
SDK/credentials file, or a secrets manager like AWS Secrets Manager/HashiCorp
Vault) and ensure the code reads the secret at runtime rather than embedding it,
remove the secret from the repository history and add it to .gitignore if
applicable, rotate and revoke the exposed AWS key immediately, and add a comment
or configuration check that fails fast if no secure credential source is
available.


class UserManager {
public:
// 2. 也是硬编码,且作为类成员
string db_password = "root";
Comment on lines +16 to +17

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Hardcoded database password.

The database password is hardcoded as a public class member, creating multiple security risks:

  • Credential exposure in source code and version control
  • Public visibility allows any code accessing UserManager to read the password
  • Cannot be rotated without code changes and redeployment
🔎 Recommended secure alternatives
 class UserManager {
-public:
-    // 2. 也是硬编码,且作为类成员
-    string db_password = "root";
+private:
+    string db_password;
+
+public:
+    UserManager() {
+        const char* pw = getenv("DB_PASSWORD");
+        if (!pw) {
+            throw runtime_error("DB_PASSWORD environment variable not set");
+        }
+        db_password = pw;
+    }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In vulnerable.cpp around lines 16-17, the db_password is hardcoded as a public
class member which exposes credentials and prevents rotation; remove the
hardcoded literal and the public member, make the password private or better yet
do not store it in the class at all; load the credential at runtime from a
secure source (environment variable, configuration file with strict permissions,
or a secrets manager/vault) and inject it into the component via constructor
parameters or a factory method; ensure the secret is kept in the smallest
possible scope, never logged, and support rotation by referencing the external
secret store rather than embedding the value in source.


// 3. SQL 注入 (CWE-89)
// 直接拼接字符串构建 SQL 查询是 C++ Web 后端常见的严重漏洞
void queryUser(string userId) {
string query = "SELECT * FROM users WHERE id = '" + userId + "'";
cout << "Executing query: " << query << endl;
}
Comment on lines +19 to +24

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: SQL injection vulnerability.

Direct string concatenation of user input into SQL queries allows attackers to inject arbitrary SQL. An input like ' OR '1'='1 bypasses authentication and '; DROP TABLE users; -- destroys data.

🔎 Recommended fix using parameterized queries
     // 3. SQL 注入 (CWE-89)
-    // 直接拼接字符串构建 SQL 查询是 C++ Web 后端常见的严重漏洞
     void queryUser(string userId) {
-        string query = "SELECT * FROM users WHERE id = '" + userId + "'";
-        cout << "Executing query: " << query << endl;
+        // Use parameterized queries with a proper database library
+        // Example with a hypothetical DB library:
+        PreparedStatement stmt = db.prepare("SELECT * FROM users WHERE id = ?");
+        stmt.bind(1, userId);
+        ResultSet rs = stmt.execute();
+        cout << "Query executed safely" << endl;
     }

Always use parameterized queries or prepared statements to prevent SQL injection.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In vulnerable.cpp around lines 19 to 24, the function queryUser builds an SQL
query by concatenating untrusted userId directly into the SQL string, creating a
critical SQL injection vulnerability; replace the string concatenation with a
parameterized/prepared statement using your DB client API (prepare the statement
with a placeholder, bind the userId as a parameter, then execute) and add basic
input validation and error handling (reject or normalize unexpected formats)
instead of manual escaping.


// 4. 命令注入 (CWE-78)
// 允许外部输入直接进入 system() 函数
void pingHost(string ipAddress) {
string cmd = "ping -c 4 " + ipAddress;
system(cmd.c_str());
}
Comment on lines +26 to +31

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Command injection vulnerability.

Passing unsanitized user input to system() allows arbitrary command execution. An attacker can inject commands using shell metacharacters like ; rm -rf / or && cat /etc/passwd.

🔎 Recommended fix using safe alternatives
     // 4. 命令注入 (CWE-78)
-    // 允许外部输入直接进入 system() 函数
     void pingHost(string ipAddress) {
-        string cmd = "ping -c 4 " + ipAddress;
-        system(cmd.c_str());
+        // Validate input format
+        regex ip_pattern(R"(^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$)");
+        if (!regex_match(ipAddress, ip_pattern)) {
+            cerr << "Invalid IP address format" << endl;
+            return;
+        }
+        
+        // Use execve family instead of system() to avoid shell interpretation
+        pid_t pid = fork();
+        if (pid == 0) {
+            execl("/bin/ping", "ping", "-c", "4", ipAddress.c_str(), NULL);
+            exit(1);
+        }
+        waitpid(pid, NULL, 0);
     }

Never use system() with user input. Use execve family functions and validate all inputs.

Based on static analysis hints, system() calls enable command stacking and should be avoided.

🧰 Tools
🪛 ast-grep (0.40.3)

[warning] 29-29: Don't call system. It's a high-level wrapper that allows for stacking multiple commands. Always prefer a more restrictive API such as calling execve from the exec family.
Context: system(cmd.c_str());
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS
Command ('OS Command Injection').
[REFERENCES]
- https://owasp.org/Top10/A03_2021-Injection

(dont-call-system-cpp)

🤖 Prompt for AI Agents
In vulnerable.cpp around lines 26 to 31, the code constructs a shell command
with untrusted input and calls system(), allowing command injection; replace
this by rejecting or sanitizing input and invoking the ping functionality
without a shell. Validate the ipAddress strictly (e.g., allow only IPv4 digits
and dots or canonicalize/parse with inet_pton/getaddrinfo), and then call a
non-shell execution API (fork+execve/execv/execvp or a platform-specific ping
library) passing the IP as a single argv element so no shell metacharacters are
interpreted; alternatively implement ICMP echo using sockets (raw socket or a
library) to avoid external process calls entirely. Ensure errors on invalid
input return clean failure and never concatenate user input into a shell command
string.

};

void legacyBufferOverflow(char* input) {
char buffer[10];

// 5. 经典的栈缓冲区溢出 (CWE-121)
// 虽然是 C++,但混用 C 风格字符串处理函数非常常见且危险
strcpy(buffer, input);
}
Comment on lines +34 to +40

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Buffer overflow vulnerability.

Using strcpy without bounds checking on a fixed-size buffer allows attackers to overflow the stack. Any input longer than 9 bytes (plus null terminator) will overwrite adjacent memory, potentially corrupting the return address for arbitrary code execution.

🔎 Recommended fix using safe string functions
 void legacyBufferOverflow(char* input) {
     char buffer[10];
     
-    // 5. 经典的栈缓冲区溢出 (CWE-121)
-    // 虽然是 C++,但混用 C 风格字符串处理函数非常常见且危险
-    strcpy(buffer, input); 
+    // Use strncpy with proper null termination, or better yet, use std::string
+    strncpy(buffer, input, sizeof(buffer) - 1);
+    buffer[sizeof(buffer) - 1] = '\0';
+    
+    // Best practice: use C++ strings instead of C-style char arrays
+    // string safeBuffer = string(input).substr(0, 9);
 }
🤖 Prompt for AI Agents
In vulnerable.cpp around lines 34 to 40, the function legacyBufferOverflow uses
strcpy into a fixed 10-byte stack buffer causing a classic stack buffer
overflow; replace the unsafe call by performing a bounded copy and ensuring
null-termination or by switching to safer types: either (a) check the input
length first and copy at most sizeof(buffer)-1 bytes then explicitly set
buffer[sizeof(buffer)-1] = '\0', or (b) use a std::string (or std::vector<char>)
to hold the input instead of a fixed C array, or (c) use a safe bounded API such
as strnlen + memcpy/strncpy/strlcpy ensuring termination; also consider changing
the parameter to const char* and validate nullptr before copying.


void memoryLeakAndRawPointers() {
// 6. 内存泄漏 (CWE-401)
// 使用了 new 但没有 delete
int* data = new int[100];
data[0] = 10;

// 抛出异常可能导致 delete 永远不执行 (异常安全问题)
if (data[0] == 10) {
// throw runtime_error("Error occurred!");
return;
}

delete[] data;
}
Comment on lines +42 to +55

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Memory leak on early return path.

The allocated array at line 45 is never freed when the early return at line 51 executes. This causes a memory leak that accumulates with repeated calls, eventually exhausting system memory.

🔎 Recommended fix using RAII
 void memoryLeakAndRawPointers() {
-    // 6. 内存泄漏 (CWE-401)
-    // 使用了 new 但没有 delete
-    int* data = new int[100];
+    // Use smart pointers for automatic memory management
+    unique_ptr<int[]> data(new int[100]);
     data[0] = 10;
     
-    // 抛出异常可能导致 delete 永远不执行 (异常安全问题)
     if (data[0] == 10) {
-        // throw runtime_error("Error occurred!"); 
         return; 
     }
-    
-    delete[] data;
+    // unique_ptr automatically cleans up on all exit paths
 }

Or use std::vector<int> data(100) to avoid manual memory management entirely.

Based on static analysis hints, Cppcheck confirms the memory leak at line 51.

🧰 Tools
🪛 Cppcheck (2.19.0)

[error] 51-51: Memory leak

(memleak)

🤖 Prompt for AI Agents
In vulnerable.cpp around lines 42 to 55, the function memoryLeakAndRawPointers
leaks the heap array allocated at line 45 on the early return at line 51;
replace manual new/delete with RAII (prefer std::vector<int> data(100) or use
std::unique_ptr<int[]>), or ensure deletion on all control paths (including
exceptions) by wrapping the allocation in a scoped owner so the memory is
automatically freed before any return or when an exception is thrown.


void iteratorInvalidation() {
vector<int> numbers = {1, 2, 3, 4, 5};

// 7. 迭代器失效 (CWE-835/Logic Error)
// 在遍历 vector 时进行 push_back 会导致底层数组重新分配,
// 从而使迭代器失效,导致未定义行为或崩溃。
for (auto it = numbers.begin(); it != numbers.end(); ++it) {
if (*it == 3) {
numbers.push_back(6);
}
}
}
Comment on lines +57 to +68

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Iterator invalidation causing undefined behavior.

Calling push_back at line 65 during iteration invalidates the iterator it. When push_back causes reallocation, the iterator becomes dangling, and subsequent increments trigger undefined behavior including crashes, infinite loops, or silent corruption.

🔎 Recommended fix
 void iteratorInvalidation() {
     vector<int> numbers = {1, 2, 3, 4, 5};
 
-    // 7. 迭代器失效 (CWE-835/Logic Error)
-    // 在遍历 vector 时进行 push_back 会导致底层数组重新分配,
-    // 从而使迭代器失效,导致未定义行为或崩溃。
-    for (auto it = numbers.begin(); it != numbers.end(); ++it) {
-        if (*it == 3) {
-            numbers.push_back(6); 
+    // Collect modifications first, apply after iteration
+    bool shouldAdd = false;
+    for (auto it = numbers.begin(); it != numbers.end(); ++it) {
+        if (*it == 3) {
+            shouldAdd = true;
         }
     }
+    if (shouldAdd) {
+        numbers.push_back(6);
+    }
 }

Never modify a container while iterating over it. Collect changes first or use index-based iteration with size caching.

Based on static analysis hints, Cppcheck flags the invalid container iterator at line 63.

🧰 Tools
🪛 Cppcheck (2.19.0)

[error] 63-63: Using iterator to local container 'numbers' that may be invalid.

(invalidContainer)

🤖 Prompt for AI Agents
In vulnerable.cpp around lines 57 to 68, the loop performs numbers.push_back(6)
while iterating, which can reallocate the vector and invalidate the iterator; to
fix, avoid mutating the container during iteration—either (a) collect values to
append in a separate container and call numbers.insert/numbers.push_back after
the loop, or (b) use an index-based loop caching the original size (size_t n =
numbers.size(); for (size_t i = 0; i < n; ++i) { if (numbers[i] == 3)
numbers.push_back(6); }) so push_back appends beyond the cached range and does
not affect current iteration.


void weakRandomness() {
// 8. 弱伪随机数生成器 (CWE-338)
// srand/rand 不适合用于安全相关的随机数生成
srand(time(NULL));
int token = rand();
cout << "Security Token: " << token << endl;
}
Comment on lines +70 to +76

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Weak randomness for security-critical token generation.

Using rand()/srand() with time(NULL) for security tokens is cryptographically weak:

  • rand() is a linear congruential generator, not cryptographically secure
  • time(NULL) provides only second-level granularity, easily predictable
  • Attackers can enumerate possible token values

Additionally, printing the token to stdout at line 75 exposes it in logs.

🔎 Recommended fix using cryptographically secure RNG
+#include <random>
+
 void weakRandomness() {
-    // 8. 弱伪随机数生成器 (CWE-338)
-    // srand/rand 不适合用于安全相关的随机数生成
-    srand(time(NULL));
-    int token = rand(); 
-    cout << "Security Token: " << token << endl;
+    // Use cryptographically secure random number generator
+    random_device rd;
+    mt19937 gen(rd());
+    uniform_int_distribution<> dis(0, INT_MAX);
+    int token = dis(gen);
+    
+    // For production: use std::random_device alone or platform-specific CSPRNGs
+    // Never log security tokens
+    // cout << "Security Token: " << token << endl;
 }

For production security tokens, use platform-specific CSPRNGs like /dev/urandom on Unix or CryptGenRandom on Windows.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In vulnerable.cpp around lines 70 to 76, the function weakRandomness uses
srand(time(NULL)) and rand() to generate a security token and prints it to
stdout, which is predictable and leaks secrets; replace this with a
cryptographically secure RNG (platform-appropriate) such as reading from
/dev/urandom or using std::random_device combined with
std::uniform_int_distribution (or Windows CryptGenRandom / BCryptGenRandom) to
generate the token, remove the use of srand/time(NULL), stop printing the token
to stdout (return it or store it securely and log only non-sensitive metadata),
and handle/read errors from the CSPRNG source appropriately.


int main(int argc, char* argv[]) {
UserManager um;

if (argc < 2) {
return 1;
}

// 模拟攻击路径
um.queryUser(argv[1]); // 传入 "' OR '1'='1" 即可注入

legacyBufferOverflow(argv[1]);

memoryLeakAndRawPointers();

iteratorInvalidation();

weakRandomness();

// 9. 被除数为零 (CWE-369)
int x = 0;
int y = 100 / x;

return 0;
}
Comment on lines +78 to +101

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Division by zero causes guaranteed crash.

Line 98 performs division by zero, causing immediate program termination. Variable x is initialized to 0 at line 97, making this crash deterministic.

Additionally, the minimal validation at line 81 doesn't prevent exploitation of the vulnerabilities throughout the function. User input argv[1] is passed unchecked to:

  • SQL injection vector (line 86)
  • Buffer overflow vector (line 88)
🔎 Recommended fixes
 int main(int argc, char* argv[]) {
     UserManager um;
 
     if (argc < 2) {
+        cerr << "Usage: " << argv[0] << " <input>" << endl;
         return 1;
     }
 
-    // 模拟攻击路径
-    um.queryUser(argv[1]); // 传入 "' OR '1'='1" 即可注入
+    // Add input validation and proper error handling
+    try {
+        um.queryUser(argv[1]);
+        legacyBufferOverflow(argv[1]);
+        memoryLeakAndRawPointers();
+        iteratorInvalidation();
+        weakRandomness();
+    } catch (const exception& e) {
+        cerr << "Error: " << e.what() << endl;
+        return 1;
+    }
     
-    legacyBufferOverflow(argv[1]);
-    
-    memoryLeakAndRawPointers();
-    
-    iteratorInvalidation();
-    
-    weakRandomness();
-
-    // 9. 被除数为零 (CWE-369)
-    int x = 0;
-    int y = 100 / x; 
+    // Fix division by zero
+    int x = 1;  // or validate x != 0 before division
+    int y = 100 / x;
 
     return 0;
 }

Based on static analysis hints, Cppcheck confirms the division by zero at line 98.

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Cppcheck (2.19.0)

[error] 98-98: Division by zero.

(zerodiv)

🤖 Prompt for AI Agents
In vulnerable.cpp around lines 78 to 101, the code performs an unconditional
division by x which is set to 0 (lines 97–98) causing a guaranteed crash; also
argv[1] is passed unchecked into functions that can be exploited (SQL injection
at line 86 and buffer overflow at line 88). Fix by adding a runtime check that x
is non‑zero before performing the division and handle the error path (return
error code or skip division) instead of dividing by zero; validate and sanitize
argv[1] before passing it to queryUser and legacyBufferOverflow (e.g., enforce
length limits, reject or escape dangerous characters, or use safe APIs) and
perform bounds checks in legacyBufferOverflow or replace it with safe buffer
handling to prevent overflow.