Better Error Handling

[safesploit]

A discussion around ways to improve error handling within Doogle

[safesploit]

Config.php

The script is currently set to output an error message if it can't connect to the database. This could potentially reveal sensitive information about your database or server configuration. It would be better to log the error message and show a generic error page to the user.

Current approach

catch (PDOException $e) {
    echo "Connection failed: " . $e->getMessage();
}

The problem with this approach is that it directly outputs the error message to the user. If there's a problem with the database connection, the error message could contain information about your database or server configuration, which could be helpful to an attacker.

Revise Error Handling

In this updated code:

1. The error_log() function sends the error message to the server's error log. This allows tracking of errors without exposing any sensitive information to the user. Make sure the server is configured to properly secure its error logs.
2. The include('error_page.php'); line includes a generic error page. This page should provide a user-friendly message indicating that an error occurred, but it should not contain any sensitive information. The following will need to be created error_page.php file.
3. The exit; statement stops the execution of the script. This prevents any further output, which could potentially contain sensitive information.

catch (PDOException $e) {
    error_log("Connection failed: " . $e->getMessage());
    include('error_page.php');
    exit;
}

Remember, it's important to test your error handling to ensure that it works correctly and does not reveal any sensitive information ;)

[safesploit]

Database Queries assumed successful

Right now, it's assumed that all database queries will succeed. However, there could be issues with the queries, such as syntax errors or issues with the data being inserted or updated. You should check the result of each database operation and handle any errors. The PDO errorInfo() method can be used to get information about errors.

The PHP Data Objects (PDO) extension provides the errorInfo() method, which can be used to get information about errors that occur during database operations.

public function getResultsHtml($page, $pageSize, $term) {
    $query = $this->con->prepare("SELECT * 
                                  FROM sites 
                                  WHERE title LIKE :term 
                                  OR url LIKE :term 
                                  OR keywords LIKE :term 
                                  OR description LIKE :term
                                  ORDER BY clicks DESC
                                  LIMIT :fromLimit, :pageSize");
    $searchTerm = '%' . $term . '%';
    $fromLimit = ($page - 1) * $pageSize;
    $query->bindParam(":term", $searchTerm);
    $query->bindParam(":fromLimit", $fromLimit, PDO::PARAM_INT);
    $query->bindParam(":pageSize", $pageSize, PDO::PARAM_INT);

    $result = $query->execute();

    if (!$result) {
        $errorInfo = $this->con->errorInfo();
        error_log("SQL Error: " . $errorInfo[2]);
        return "<div class='alert alert-danger'>There was a problem retrieving the search results. Please try again later.</div>";
    }

    // Existing code to generate and return the HTML...
}

[safesploit]

File includes

The include statement is used to include other PHP files. If these files can't be found or can't be read for some reason, this will cause a warning. Consider include_once or require_once instead, which will cause a fatal error if the file can't be included. This is generally better because it prevents the script from continuing to execute with missing code.

require_once("classes/SiteResultsProvider.php");

Explanation of include, include_once, require and require_once

When you use include, if the file can't be found or read for some reason, PHP will generate a warning, but it will continue to execute the rest of the script. This can lead to unexpected behavior if the rest of the script depends on the included file.

The include_once statement is similar to include, but it will check if the file has already been included, and if so, it will not include it again. This can be useful to prevent problems with function redefinitions, variable value reassignments, etc.

The require statement is also similar to include, but it behaves differently when the file can't be found or read. Instead of just generating a warning like include, require will generate a fatal error and stop the script. This can be better than include if the rest of the script can't run correctly without the included file.

The require_once statement is a combination of require and include_once: it will generate a fatal error if the file can't be found or read, and it will check if the file has already been included, and if so, it won't include it again.

[safesploit]

User Input

The search term provided by the user is directly used in your SQL queries. While using prepared statements helps to prevent SQL injection, validating and sanitising this input to prevent other types of issues. The PHP filter_var() function could be used for this purpose.

Issue

• Users might accidentally or intentionally enter data that your script doesn't expect, which can cause errors or unexpected behavior.
• Malicious users might try to exploit your script by providing input that includes special characters, unusually large or small values, or other unexpected data.

Migration

• Validation means checking that the input meets certain criteria.
• Sanitisation means cleaning up the input to remove any potentially harmful or unexpected data. This could involve removing or escaping special characters, trimming excess whitespace, converting the input to a certain data type, etc.

PHP provides several functions that can be used to validate and sanitise input. One of these is the filter_var function, which can both validate and sanitize input. Here's an example of how you might use filter_var to sanitise the search term in your search.php file:

$term = filter_var($_GET["term"], FILTER_SANITIZE_STRING);

In this example, FILTER_SANITIZE_STRING is a filter that removes tags and encodes special characters from the search term. This can help prevent issues like cross-site scripting (XSS) attacks, where an attacker injects malicious code into your page via user input.

[safesploit]

Removing HTML tags: The strip_tags() function can be used to remove all HTML and PHP tags from a string. This can be useful to prevent cross-site scripting (XSS) attacks, where an attacker injects malicious code into your page via user input.

$sanitised_input = strip_tags($input);

[safesploit]

Escaping output: The htmlspecialchars() function can be used to convert special characters to their HTML entities. This means that they will be displayed as they are, instead of being interpreted as HTML. This can also help to prevent XSS attacks.

echo htmlspecialchars($input, ENT_QUOTES, 'UTF-8');

[safesploit]

Regular expressions: The preg_replace() function can be used to search for and replace patterns in a string using regular expressions. This can be used to remove or replace specific characters or sequences of characters.

// Remove non-alphanumeric characters
$sanitized_input = preg_replace("/[^a-zA-Z0-9]/", "", $input);

[safesploit]

Type casting: If you expect an input to be a certain type, such as an integer, you can cast the input to that type. This will convert the input to that type, or give it a default value if the conversion isn't possible.

$integer_input = (int)$input;

[safesploit]

Trimming: The trim() function can be used to remove whitespace from the beginning and end of a string. This can be useful to prevent issues with unexpected whitespace.

$trimmed_input = trim($input);

[safesploit]

File and Directory Permissions

While less important for Doogle, this proved to be an ongoing issue with Valhalla2 which allowed users to upload files into the web root.

In PHP, file and directory permissions can affect whether a script can read from or write to a file or directory. If the script needs to access a file or directory that it doesn't have permission to access, this will cause an error.

Before trying to read from or write to a file, you can use the is_readable() and is_writable() functions to check whether the file is readable or writable, respectively. Here's an example:

$filename = "/path/to/your/file.txt";

if (is_readable($filename)) {
    // The file is readable, so you can proceed with reading from the file
    $file_contents = file_get_contents($filename);
} else {
    // The file is not readable, so handle the error
    error_log("File is not readable: " . $filename);
    // You could also display a user-friendly error message, or handle the error in another way
}

if (is_writable($filename)) {
    // The file is writable, so you can proceed with writing to the file
    file_put_contents($filename, "Hello, world!");
} else {
    // The file is not writable, so handle the error
    error_log("File is not writable: " . $filename);
    // You could also display a user-friendly error message, or handle the error in another way
}

Remember to replace "/path/to/your/file.txt" with the actual path to the file you want to access.

Keep in mind that file and directory permissions can depend on the user that your web server runs as. For example, on a Unix-like system, if your web server runs as the "www-data" user, then that user needs to have read and/or write access to the file or directory.