Secure Code Review: A journey from flaws to fortifying code — Part 2(CSRF, IDOR, File Upload)

Aakash Mudigonda
13 min read1 day ago

--

Hello fellow Developers and hackers! I bring to you the 2nd part of getting started into Secure Code Review, and today we will discuss about 3 code vulnerabilities and how to, exploit them, fix them, why the exploit won’t work after putting necessary remediations in the code.

Should we get started then?

CSRF:

A web attack, which tricks users to perform actions without their knowledge!

Lets say you have already logged into your bank application. I get to know this somehow, and I also know that there is a CSRF vulnerability in the bank application. I now forge a request, to automatically update your bank’s email address to mine. So, when you click on the link I send, you unknowingly change your email address. And just like that, I own the account now.

This works, because the bank application knows you have already logged in, and it assumes you are consciously transferring the money to me.
Poor you :(

Vulnerable Code (CSRF):

<?php
session_start();
// Assume user is logged in and their user ID is stored in the session

if (isset($_POST['email'])) {
$email = $_POST['email'];

$stmt = $conn->prepare("UPDATE users SET email = ? WHERE id = ?");
$stmt->bind_param("si", $email, $_SESSION['user_id']);
$stmt->execute();

echo "Email updated!";
}
?>

Try to exploit this code, without scrolling further down.

Exploitation (CSRF):

<html>
<body>
<form action="http://vulnerable-site.com/update_email.php" method="POST" id="csrfForm">
<!-- The attacker sets the email to their own address -->
<input type="hidden" name="email" value="attacker@example.com">
</form>
<script>
// Auto-submit the form when the page loads
document.getElementById("csrfForm").submit();
</script>
</body>
</html>

An attacker creates a malicious page that automatically submits a POST request to change the user’s email address.

When a logged-in user visits the attacker’s page, their browser automatically sends the POST request to the vulnerable script, updating their email address without their consent.

Mitigated Code (CSRF) :

To defend against CSRF, you can implement a CSRF token mechanism. The token is a unique value generated for the user session and included with each form submission. Here’s the mitigated version of the code:

<?php
session_start();

// Generate a CSRF token if one does not exist
if (empty($_SESSION['csrf_token'])) {
$_SESSION['csrf_token'] = bin2hex(random_bytes(32));
}

// When processing the form submission:
if (isset($_POST['email']) && isset($_POST['csrf_token'])) {
// Validate the CSRF token using a timing-attack safe comparison
if (hash_equals($_SESSION['csrf_token'], $_POST['csrf_token'])) {
$email = $_POST['email'];

// Safe to perform the update now
$stmt = $conn->prepare("UPDATE users SET email = ? WHERE id = ?");
$stmt->bind_param("si", $email, $_SESSION['user_id']);
$stmt->execute();

echo "Email updated!";
} else {
// Token did not match
echo "Invalid CSRF token.";
}
}
?>

In your HTML form (our bank website) for updating the email, include the CSRF token as a hidden field — Developers take notes! :

<form action="update_email.php" method="POST">
<input type="email" name="email" placeholder="Enter your new email" required>
<input type="hidden" name="csrf_token" value="<?php echo $_SESSION['csrf_token']; ?>">
<button type="submit">Update Email</button>
</form>

Ideally you should be adding an additional check by sending a 2FA — Like a one time password to the user, and asking them to input it, and only if that matches and your CSRF Token matches, you should update the email. I’ll discuss this in a future post!

Explanation of Mitigated Code

Session Initialization and Token Generation:

session_start();

if (empty($_SESSION['csrf_token'])) {
$_SESSION['csrf_token'] = bin2hex(random_bytes(32));
}

Starts the session and checks if a CSRF token exists or not. If not, generates a new random token and stores it in the session

Processing the form with CSRF Token Verification:

if (isset($_POST['email']) && isset($_POST['csrf_token'])) {
if (hash_equals($_SESSION['csrf_token'], $_POST['csrf_token'])) {
// Process update
} else {
echo "Invalid CSRF token.";
}
}
  • Checks if both the email and CSRF token are submitted.
  • Uses hash_equals to securely compare the token from the form ($_POST['csrf_token']) with the one stored in the session ($_SESSION['csrf_token']).
  • Only if the tokens match does it proceed with updating the user’s email.

Database Update:

$stmt = $conn->prepare("UPDATE users SET email = ? WHERE id = ?");
$stmt->bind_param("si", $email, $_SESSION['user_id']);
$stmt->execute();

Uses a prepared statement with bound parameters to safely update the user’s email in the database, mitigating SQL injection risks as well.

Including the token in the HTML Form:

When the server sends the HTML form to the user’s browser, it embeds the token into a hidden input field:

<input type="hidden" name="csrf_token" value="<?php echo $_SESSION['csrf_token']; ?>">

This means the token becomes part of the form data that will be submitted later.

Trying to exploit Mitigated Code (CSRF):

Attacker’s code:

<html>
<body>
<form action="http://vulnerable-site.com/update_email.php" method="POST" id="csrfForm">
<input type="hidden" name="email" value="attacker@example.com">
<!-- Attacker cannot guess the valid CSRF token, so they might try a dummy token -->
<input type="hidden" name="csrf_token" value="fake_token">
</form>
<script>
document.getElementById("csrfForm").submit();
</script>
</body>
</html>

In the Mitigated Code:

  • The server receives the POST request with csrf_token=fake_token.
  • During processing, the server compares fake_token with the legitimate token stored in the session.
  • The hash_equals check fails because the tokens do not match.

Why the exploit won’t work:

  • Token Validation: The CSRF token generated for the session is random and unpredictable. The attacker does not have access to this token.
  • Mismatch Detection: Since the token in the malicious request does not match the valid token stored in the user’s session, the server refuses to process the request.
  • Request Integrity: Only requests that originate from a page on your own site (with the correct CSRF token) are accepted. This separation ensures that even if a user is tricked into visiting an attacker-controlled page, the forged request will not include the correct token.

IDOR :

IDOR — Insecure Direct Object Reference. occurs when an application exposes internal implementation objects (such as database records or files) directly to the user.

Attackers can manipulate input (such as a query parameter) to access or modify data that they shouldn’t be able to.

Lets say a website uses the following URL to access the customer account page, by retrieving information from the back-end database:

https://insecure-website.com/customer_account?customer_number=132355htt

Here, the customer number is used directly as a record index in queries that are performed on the back-end database.

If no other controls are in place, an attacker can simply modify the customer_number value, bypassing access controls to view the records of other customers. This is an example of an IDOR vulnerability leading to privilege escalation.

An attacker might be able to perform horizontal and vertical privilege escalation by altering the user to one with additional privileges while bypassing access controls. Other possibilities include exploiting password leakage or modifying parameters once the attacker has landed in the user’s accounts page, for example.

Vulnerable Code (IDOR):

<?php
session_start();

$requested_user_id = $_GET['user_id'];

$stmt = $conn->prepare("SELECT * FROM users WHERE id = ?");
$stmt->bind_param("i", $requested_user_id);
$stmt->execute();
$result = $stmt->get_result();
$user_data = $result->fetch_assoc();

// Display user information
echo "Name: " . $user_data['name'];
?>

Try exploiting the code, without scrolling further below!

Exploitation (IDOR):

Asusme a scenario, where a user is logged in with a session user ID of 3. The application URL is something like:

https://example.com/profile.php?user_id=3

An attacker notices that the user ID is passed in the URL.

Attack Example:

The attacker modifies the URL to:

https://example.com/profile.php?user_id=4

If user 4 is another valid user, the attacker will see that user’s profile information — even though they should only have access to their own data.

Mitigated Code (IDOR) :

The application should ensure that the user can only access their own data. One simple approach is to compare requested USER ID with the USER ID stored in the session. Doing this would mitigate the IDOR Vulnerability:

<?php
session_start();
// Assume the logged-in user's ID is stored in the session
$session_user_id = $_SESSION['user_id'];
$requested_user_id = $_GET['user_id'];

// Mitigation: Ensure the requested user ID matches the session's user ID
if ($requested_user_id != $session_user_id) {
die("Access denied.");
}

// If the IDs match, proceed to fetch and display the user data
$stmt = $conn->prepare("SELECT * FROM users WHERE id = ?");
$stmt->bind_param("i", $requested_user_id);
$stmt->execute();
$result = $stmt->get_result();
$user_data = $result->fetch_assoc();

echo "Name: " . $user_data['name'];
?>

Explanation of the code:

Session Initialization and Retrieval of IDs:

session_start();
$session_user_id = $_SESSION['user_id'];
$requested_user_id = $_GET['user_id'];
  • We initialize the session and set the session id and user id
  • Retrieve the User ID from the session
  • Retrieve the User ID — from the request of the user (ex: URL)

Access Control Check:

if ($requested_user_id != $session_user_id) {
die("Access denied.");
}
  • Compares the User ID requested by the user with the User ID stored in the session.
  • If both do not match, the script stops with an Access Denied message.
  • This check prevents users from viewing or manipulating other users’ data

Database Query and Data Display:

$stmt = $conn->prepare("SELECT * FROM users WHERE id = ?");
$stmt->bind_param("i", $requested_user_id);
$stmt->execute();
$result = $stmt->get_result();
$user_data = $result->fetch_assoc();

echo "Name: " . $user_data['name'];

We use a prepared statement to safely retrieve the user’s data

Trying to Exploit the Mitigated Code (IDOR):

Attacker has the user session id as “3” but wants to access user id : “4

https://example.com/profile.php?user_id=4

What Happens in the Mitigated Code:

  • The code retrieves:
  • $session_user_id = 3
  • $requested_user_id = 4

The access control check:

if ($requested_user_id != $session_user_id) {
die("Access denied.");
}

Finds that 4 is not equal to 3, so the script stops executing and outputs “Access denied.”

The exploit doesn’t work here because of:

  • Enforced Access Control: The mitigation explicitly checks that the user ID in the URL matches the user ID in the session.
  • No Unauthorized Data Access: If an attacker tries to change the user ID to one that does not belong to them, the condition fails, and the script refuses to process the request.
  • Prevents Data Leakage: As a result, only the authenticated user’s own data is retrieved and displayed, protecting the privacy and integrity of other users’ information.

Down to the last one!

File Upload Vulnerability:

This vulnerability occurs when, application allows users to upload files without proper validation. This can enable attackers to upload malicious files (such as web shells, scripts, or malware) that may be executed on the server.

Vulnerable Code (File Upload):

<?php
if (isset($_FILES['upload'])) {
$upload_dir = 'uploads/';
$upload_file = $upload_dir . basename($_FILES['upload']['name']);

if (move_uploaded_file($_FILES['upload']['tmp_name'], $upload_file)) {
echo "File uploaded successfully!";
} else {
echo "File upload failed!";
}
}
?>

Try to exploit this code, without scrolling further below!

Trying to exploit the code (File Upload):

An attacker exploits the file upload functionality to upload a malicious PHP script (e.g., shell.php).

The shell.php file contains the following:

<?php system($_GET['cmd']); ?>

If the file is uploaded to a web-accessible directory (like uploads/), the attacker can execute the script by navigating to:

https://example.com/uploads/shell.php?cmd=ls

This command could allow the attacker to execute arbitrary commands on the server.

Mitigated Code (File Upload):

<?php
if (isset($_FILES['upload'])) {
$upload_dir = 'uploads/';

// Allowed file extensions
$allowed_ext = ['jpg', 'jpeg', 'png', 'gif', 'pdf'];

// Extract file extension
$file_info = pathinfo($_FILES['upload']['name']);
$file_ext = strtolower($file_info['extension']);

// Validate file extension and size (e.g., max 2MB)
if (in_array($file_ext, $allowed_ext) && $_FILES['upload']['size'] <= 2 * 1024 * 1024) {

// Sanitize filename (e.g., remove special characters)
$safe_filename = preg_replace("/[^a-zA-Z0-9_-]/", "", $file_info['filename']);

// Create a unique file name to prevent overwriting
$new_filename = $safe_filename . '_' . time() . '.' . $file_ext;
$upload_file = $upload_dir . $new_filename;

if (move_uploaded_file($_FILES['upload']['tmp_name'], $upload_file)) {
echo "File uploaded successfully!";
} else {
echo "File upload failed!";
}
} else {
echo "Invalid file type or file too large!";
}
}
?>

File Extension Validation:

$allowed_ext = ['jpg', 'jpeg', 'png', 'gif', 'pdf'];
$file_info = pathinfo($_FILES['upload']['name']);
$file_ext = strtolower($file_info['extension']);

Purpose:

  • Define a list of allowed file extensions.
  • Extract and normalize the uploaded file’s extension.

Advantage:

  • Ensures that only files with acceptable extensions (e.g., images or PDFs) are permitted.

File Size Check:

if (in_array($file_ext, $allowed_ext) && $_FILES['upload']['size'] <= 2 * 1024 * 1024) {

Purpose:

  • Confirms that the file size does not exceed a specified limit (here, 2MB).

Advantage:

  • Prevents large files from being uploaded, which can exhaust server resources.

Filename Sanitization:

$safe_filename = preg_replace("/[^a-zA-Z0-9_-]/", "", $file_info['filename']);

Purpose:

  • Removes special characters from the original filename to avoid directory traversal attacks or file system issues.

Advantage:

  • Creates a sanitized filename that minimizes risk.

Unique Filename Generation:

$new_filename = $safe_filename . '_' . time() . '.' . $file_ext;

Purpose:

  • Appends the current timestamp to the sanitized filename to ensure uniqueness.

Advantage:

  • Prevents filename collisions and accidental overwriting of files.

Secure File Upload:

if (move_uploaded_file($_FILES['upload']['tmp_name'], $upload_file)) {
echo "File uploaded successfully!";
}

Purpose:

  • Moves the uploaded file from the temporary directory to the designated upload directory securely.

Advantage:

  • Ensures that only validated files are stored on the server.

Welp, that’s it — Wait! The code above! Is it secure? Are you sure? Check it again!

confused?

Though the code looks secure, if the attacker puts in 2 extensions, file.php.jpg then the code still accepts it!

It still executes the php code then!

What do I now Aakash?

Dont worry, I got your back!

Here you go with an even better mitigated code:

Mitigated Code 2.0 (File Upload):

<?php
if (isset($_FILES['upload'])) {
$upload_dir = 'uploads/';

// Define allowed file extensions and corresponding MIME types
$allowed_ext = ['jpg', 'jpeg', 'png', 'gif', 'pdf'];
$allowed_mime = ['image/jpeg', 'image/png', 'image/gif', 'application/pdf'];

// Extract file information from the uploaded file
$file_info = pathinfo($_FILES['upload']['name']);
$file_ext = strtolower($file_info['extension']);

// 1. Validate file extension
if (!in_array($file_ext, $allowed_ext)) {
die("Invalid file extension!");
}

// 2. Validate MIME type using PHP's FileInfo extension
$finfo = finfo_open(FILEINFO_MIME_TYPE);
$mime_type = finfo_file($finfo, $_FILES['upload']['tmp_name']);
finfo_close($finfo);

if (!in_array($mime_type, $allowed_mime)) {
die("Invalid file type!");
}

// 3. (Optional) For image files, verify that they are valid images
if (in_array($file_ext, ['jpg', 'jpeg', 'png', 'gif'])) {
$image_info = getimagesize($_FILES['upload']['tmp_name']);
if ($image_info === false) {
die("File is not a valid image!");
}
}

// 4. Sanitize and generate a unique filename to remove any malicious double extension attempt
$safe_filename = preg_replace("/[^a-zA-Z0-9_-]/", "", $file_info['filename']);
$new_filename = $safe_filename . '_' . time() . '.' . $file_ext;
$upload_file = $upload_dir . $new_filename;

// 5. Move the uploaded file from its temporary location to the desired directory
if (move_uploaded_file($_FILES['upload']['tmp_name'], $upload_file)) {
echo "File uploaded successfully as " . htmlspecialchars($new_filename);
} else {
echo "File upload failed!";
}
}
?>

Allowed File Extensions and MIME Types:

  • We define a whitelist of acceptable file extensions ($allowed_ext) and corresponding MIME types ($allowed_mime). This ensures that only expected file types are processed.
  • A MIME type (also known as a Multipurpose Internet Mail Extension) is a standard that indicates the format of a file

File Extension Validation:

  • The code uses pathinfo to extract the file extension from the original filename. Even if an attacker uses a name like reverse.php.jpg, the extension extracted will be "jpg", which is then checked against the allowed list.

MIME Type Validation:

  • Using the FileInfo extension (finfo_open and finfo_file), the actual MIME type of the file is determined. This helps verify that the file content matches the expected type. Even if the filename is deceptive, the MIME type check will reveal if a file is not a genuine image or PDF.

Optional Image Verification:

  • For image files, getimagesize is used as an extra check to confirm that the file is a valid image. This helps to further filter out any malicious files that might be masquerading as images.

Filename Sanitization and Unique Naming:

  • The original filename is sanitized by removing unwanted characters.
  • A unique filename is generated by appending the current timestamp. This approach not only prevents issues with double extensions but also avoids filename collisions and overwriting existing files.

Moving the File:

  • Finally, move_uploaded_file securely moves the file from the temporary directory to the designated upload directory.

Trying to exploit Mitigated Code 2.0 (File Upload):

File Name Trickery:

  • The attacker names the file reverse.php.jpg in an effort to hide malicious PHP code behind a seemingly safe image extension.

MIME Type and Content Manipulation:

The attacker then carefully edits the file so that:

  • The file header (the “magic number”) appears as a JPEG.
  • The file passes the finfo_file MIME type check (for example, it returns image/jpeg).
  • The file is structured in such a way that getimagesize() returns valid image data.

Server Misconfiguration Exploit:

  • The attacker hopes that the server is misconfigured so that if the file is later accessed via a URL in the uploads/ directory, the PHP code embedded within the image will be executed.

Now the exploit by the attacker, will not work. Why?

File Extension Check:

  • The code extracts the file extension based on the final dot.
  • In our example, reverse.php.jpg would yield "jpg", which is allowed.
  • However, this is only one layer of defense.

MIME Type Validation:

  • Using finfo_file(), the code checks that the MIME type matches one of the allowed types.
  • A well-crafted file must truly appear as a JPEG at the binary level.
  • Altering the file’s hex content without corrupting the JPEG structure is challenging.

Image Validation with getimagesize():

  • This function examines the image’s dimensions and structure.
  • If the attacker has inserted PHP code into non-image sections, there is a risk — but if the PHP code corrupts the image structure even slightly, getimagesize() is likely to fail.
  • In a well-implemented system, if getimagesize() returns valid image information, it’s a strong indication that the file is being interpreted as an image by PHP.

Filename Sanitization and Renaming:

  • The code sanitizes and renames the file to a controlled format (e.g., safe_filename_timestamp.jpg), removing any malicious hints from the original name.
  • This means that even if the original name tried to include .php, it won’t be part of the stored filename.

What you can do additionally — Server-Side Defenses:

  • Storage Outside the Web Root: If the upload directory is not web-accessible, then even if the file contains PHP code, it cannot be executed via a URL.
  • Web Server Configuration: Even if stored in a web-accessible directory, the server can be configured so that files in the upload directory are served as static content (or forced to download) rather than executed.
  • Re-encoding Images: In some implementations, you might reprocess or re-encode the image after upload (using GD or ImageMagick). This strips out any extraneous data (including any hidden PHP code) because the output image is generated afresh from the valid image data.

Well, that’s it for now folks! I hope you enjoyed and learnt something out of this article!

I struggled to get my basics right of secure code review and I hope this article helps bridge that gap!

Again, this is not the most secure code, this is to help people who have difficulty understanding code and secure code review and practices!

Ok then, see you again!

--

--

Responses (1)