Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL Injection Security Issue #322

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

d0ub1edd
Copy link

@d0ub1edd d0ub1edd commented Sep 9, 2024

This pull request addresses a critical SQL Injection vulnerability found of the application. The vulnerability allows any authenticated user to exploit the SQL query by injecting malicious SQL code, potentially leading to unauthorized data access or manipulation.

Impact: This vulnerability allows an attacker with any level of system access to execute arbitrary SQL queries. This can lead to data leakage, data corruption, or full database compromise.
Requirements for Exploitation: Attacker must be authenticated to the system but does not need elevated privileges.

Fix:
This PR is fixing the SQL Injection vulnerabilities.

@francoisjacquet
Copy link

Your PR lacks a proof of concept.

You can't rely on REMOTE_ADDR being true... it could be the wrong address due to anonymising proxies or some such trick. You can rely on it always being an IP address, so SQL injection by this path is impossible.

Source: https://stackoverflow.com/questions/2018151/ip-address-sql-injection#answer-2018561

@lodos2005
Copy link

lodos2005 commented Oct 5, 2024

unfixed code is

if ($_SERVER['HTTP_X_FORWARDED_FOR']) {
    $ip = $_SERVER['HTTP_X_FORWARDED_FOR'];
} else {
    $ip = $_SERVER['REMOTE_ADDR'];
}
DBQuery("INSERT INTO hacking_log (HOST_NAME,IP_ADDRESS,LOGIN_DATE,VERSION,PHP_SELF,DOCUMENT_ROOT,SCRIPT_NAME,MODNAME,USERNAME)
 values
('$_SERVER[SERVER_NAME]','$ip','" . date('Y-m-d') . "','$openSISVersion','$_SERVER[PHP_SELF]','$_SERVER[DOCUMENT_ROOT]','$_SERVER[SCRIPT_NAME]','$_REQUEST[modname]','" . User('USERNAME') . "')");

@francoisjacquet so if HTTP_X_FORWARDED_FOR is defined(it can be simply defined in the HTTP header) in the $ip variable , the value given by the attacker will be assigned, not the REMOTE_ADDR. This will cause a sql injection vulnerability. POC is already in the profile on @d0ub1edd. I verified that it works in the latest version.

https://github.com/d0ub1edd/CVE-Reference/blob/main/CVE-2024-46626.md

@francoisjacquet
Copy link

francoisjacquet commented Oct 5, 2024

@lodos2005 thanks, I just didn't expand the code up to the HTTP_X_FORWARDED_FOR header.

HTTP_* header can indeed be spoofed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants