OWASP A5:2017 – Broken Access Control

If you stumble across this post and are wondering what this is all about, then I recommend reading this post before following this guide. TL; DR, this post is about solving Secure Code Warrior challenges, more specifically their PHP Basic challenges. The “Broken Access Control” OWASP category is the one where most developers fail to implement secure code writing and as such is the “no. 1” category in OWASP TOP 10 2021. Explaining the vulnerability and remediation steps thereof in this category will be a bit challenging but nothing of the sort that I can’t handle easily.:) So, without further ado, let’s get started.

Table of Contents

Challenge 1 – Insecure Direct Object Reference(IDOR)

When an application allows direct access to objects via unvalidated user supplied input, it results in an Insecure Direct Object Reference(IDOR). In simple terms, IDOR is caused when an application takes user supplied input and uses it to retrieve an object(e.g., database record) without proper authorization checks. Following are some examples of IDOR:-

  • Retrieving a database record via user supplied input. For e.g., Using parameters with numeric values to directly represent records in database and using those values to fetch objects from database without proper validation.
  • Retrieving a file system resource via user supplied input. For e.g., when images are fetched via parameters like ?img=image01.jpg, then the filename can be manipulated to access some sensitive file.
  • Retrieving restricted application functionality via user supplied input. For e.g., Using user-supplied parameter values to access an application functionality only available to admins.

Locating Vulnerability

As we know IDOR vulnerability occurs due to user-supplied input, to locate the vulnerability we should be looking for functions where unvalidated user input is carried out by the application. As such, if we look into the getReports() function in PersonalCabinetController.php we’ll see that the reports are fetched via their IDs and stored in the $reports variable.

public function getReports(int $id): Response
    {
        if (CheckUserService::isTokenMatch($this->data['csrf'])
            && CheckUserService::isUserAuthorized($this->token)
        ) {
            $reportsResponse = [];
            $reports = $this->personalCabinetService->getReports($this->token, $id);

            if (!$reports) {
                foreach ($reports as $report) {
                    $reportsResponse = [
                        'id' => $report->id,
                        'name' => $report->name,
                    ];
                }

Note:- There are functions with the same name, i.e., getReports() in different classes.

But the problem is on line 111. Is the $id variable, which takes user supplied input, validated before passing it into the getReports() function of PersonalCabinetService class? If we look into the getReports() function of PersonalCabinetService.php below, we’ll see that on line 74 reports are fetched directly via user supplied report IDs and no validation is made on the value as a result of which an attacker can directly access any user’s reports.

/**
     * @param string $hashKey
     * @param int $id
     * @return false|mixed
     */
    public function getReports(string $hashKey, int $id)
    {
        if (User::getCurrentUser($hashKey)->role === User::MANAGER) {
            return Report::getReports($id);
        }

        return false;
    }

In the above function, although an check is implemented to verify the “user” is authenticated and has the ownership(MANAGER role) of the reports, there is no check implemented on the $id value and as such a malicious user can directly reference any file(object) from the database. To prevent IDOR vulnerability, one must avoid usage of direct object identifiers to retrieve data from the database. As such, an authenticated malicious user can manipulate the ID parameter and fetch records belonging to other users of the application directly as there is no access control checks applied on “id” parameter. For e.g., A hacker can use the following curl command to fetch any report of any user of the web application by just changing the value of id parameter.

curl -H "Cookie: `session cookie of hacker`" http://example.com/report.php?id=4345

Identifying Solution

The proposition of OWASP is to use a hash, salted with a value defined at application level, in place of a direct object identifier. Usage of a hash has two big advantages:-

  1. The usage of a hash totally eliminates the requirement of mapping a resource’s “front end ID” with the “real ID” in the database. Also, usage of hash helps in validating the user’s hash with the hash of the resource accessed(here “reports”) thus preventing unauthorized access to other users reports. The user’s session token must be tied to the resource’s hash in question to implement authorization checks.
  2. Now, even if the attacker were able to guess the hashing algorithm from the size of the hash id, he won’t be able to reproduce the hash to access other user’s reports as the hash is salted with a value defined at application level and not tied to the hidden value. This prevents enumeration of resources(here reports) via brute forcing attacks to guess the hash ID of a report.

I hope after reading my explanation of the proposition above, you may now have an idea of how the solution must look like. The solution must implement usage of a hash/token in place of direct object identifiers like $id. As such, only one solution fits the bill and is the desired solution.

public function getReports(): Response
    {
        if (CheckUserService::isTokenMatch($this->data['csrf'])
            && CheckUserService::isUserAuthorized($this->token)
        ) {
            $reportsResponse = [];
            $reports = $this->personalCabinetService->getReports($this->token);

            if (!$reports) {
                foreach ($reports as $report) {
                    $reportsResponse = [
                        'id' => $report->id,
                        'name' => $report->name,
                    ];
                }

The above secure code now uses a token(hash) instead of a direct object identifier($id) as was done in the vulnerable code before. Also, the getReports function of personalCabinetService class now uses a $hashkey(hash) to fetch the reports instead of a direct object identifier to fetch data from the database.

    /**
     * @param string $hashKey
     * @return false|mixed
     */
    public function getReports(string $hashKey)
    {
        if (User::getCurrentUser($hashKey)->role === User::MANAGER) {
            return Report::getReports($hashKey);
        }

        return false;
    }

In the above code, the getReports() function of Report class is used to fetch the reports from the database. Further in Report.php, the hash of the reports is validated with hash of the user to implement authorization checks. In simple terms, the secure token($hashkey) of the user is used to verify the user owns the reports before getting the reports from the database. The getReports() function of Report class can be seen below.

/**
 * @param string $hashKey
 * @param array|null $reportIDs
 * @return mixed
 */
public static function getReports(string $hashKey, ?array $reportIDs = null)
{
    $user = User::getCurrentUser($hashKey);

    if ($reportIDs !== null) {
        $query = self::$database->prepare(
            'SELECT * FROM reports WHERE user_uid=:uid AND id=:id'
        );
        foreach ($reportIDs as $reportID) {
            $query->bindValue(':uid', $user->uid);
            $query->bindValue(':id', $reportID);
            $query->execute();
        }
        return $query->fetch(PDO::FETCH_OBJ);
    }

Challenge 2 – Missing Function Level Access Control

Locating Vulnerability

This challenge is a bit easy. As the challenge is about missing access control checks in functions all we need to do is look for functions that do not implement checks on which user is executing it. As such the dropUser() function of PersonalCabinetService.php, which is an administrative function, does not implement checks on which user is allowed to execute it. As such, any user can execute the function and delete users from the database using the dropUser() function of User class.

/**
     * @param string $hashKey
     * @param Request $request
     * @return bool
     */
    public function dropUser(string $hashKey, Request $request): bool
    {
        $post = $request->getParsedBody();
        $sanitized = $this->service->sanitize($post);

        return User::dropUser($hashKey, $sanitized['email']);
    }

If we look into the dropUser() function of User class below, we’ll see that it does a good job of preventing SQL injections via prepared statements, but it doesn’t implement any checks on whether the user executing the function is an Administrator or not.

/**
     * @param string $hashKey
     * @param string $email
     * @return bool
     */
    public static function dropUser(string $hashKey, string $email): bool
    {
        $query = self::$database->prepare(
            'SELECT * FROM users JOIN token_users WHERE auth_token=:auth_token'
        );
        $query->bindValue(':auth_token', $hashKey);

        if ($query->execute()) {
            $query = self::$database->prepare(
                'DELETE FROM users WHERE email=:email'
            );
            $query->bindValue(':email', $email);
            Logger::info('Deleted user');

            return $query->execute();
        }

        Logger::alert('An unauthorized attempt to delete a user');

        return false;
    }

As a result, any authenticated attacker can delete any user from the database by simply knowing the victim’s email. For example the following curl command shows how a hacker can delete any user from the database:-

curl -H "Cookie: `session cookie of hacker`" "http://example.com/delete_user.php?email=random@user.com"

Identifying Solution

To identify the correct solution, we just need to look for a solution which implements a check whether the user is an Administrator, in a secure way. As such, only one solution verifies a user’s role via his session token and returns the dropUser() function of User class on line 38.

 /**
     * @param string $hashKey
     * @param Request $request
     * @return bool
     */
    public function dropUser(string $hashKey, Request $request): bool
    {
        $post = $request->getParsedBody();
        $sanitized = $this->service->sanitize($post);

        if ($sanitized['email'] !== null
            && $this->service->isValidEmail($sanitized['email'])
            && User::getCurrentUser($hashKey)->role === User::ADMINISTRATOR
        ) {
            return User::dropUser($hashKey, $sanitized['email']);
        }
        return false;
}

The dropUser() function of User class now also implements a check for the role of user executing the function so that users without the role of “ADMINISTRATOR” cannot carry out the delete function.

public static function dropUser(string $hashKey, string $email): bool
    {
        $query = self::$database->prepare(
            'SELECT * FROM users JOIN token_users WHERE auth_token=:auth_token'
        );
        $query->bindValue(':auth_token', $hashKey);

        if ($query->execute()) {
            $user = $query->fetch(PDO::FETCH_OBJ);

            if ($user->role === self::ADMINISTRATOR) {
                $query = self::$database->prepare(
                    'DELETE FROM users WHERE email=:email'
                );
                $query->bindValue(':email', $email);
                Logger::info('Deleted user');

                return $query->execute();
            }
        }

Challenge 3 – Missing Function Level Access Control

Locating Vulnerability

The vulnerability in this challenge is similar to the vulnerability in Challenge 2 above and as such I’m not going to waste time explaining it again and leave it for the reader to refer the challenge above properly to understand the vulnerability in this challenge. So, the vulnerable function here is the deleteReport() function of PersonalCabinetService.php, where like the challenge above an access control check for the “user” executing the function is not applied and as a result of that an user can delete the reports of any other user on the application.

public function deleteReport(string $hashKey, Request $request): bool
    {
        $post = $request->getParsedBody();
        $sanitized = $this->service->sanitize($post);

        return Report::deleteReport($hashKey, $sanitized['report_id']);
    }

Identifying Solution

As the previous challenge, we need to check whether the code enforces a check for the “role” needed to carry out the privileged operation of deleting a user’s reports. There are two roles defined in User.php, i.e., MANAGER and ADMINISTRATOR. A normal user needs to have the “MANAGER” role to access and delete his own files and the “role” must be cross checked with his session token in order to prevent unauthorized users from deleting a user’s files. Now, I’ll leave this as challenge for myself and anybody else referring this in the future to figure out the solution. πŸ™‚

Conclusion

I really loved writing about the IDOR challenge. This OWASP category is one where I didn’t have to depend on search engines to learn about the vulnerabilities. I just simply used official PHP docs and OWASP testing and prevention guides to solve the challenges. Now, four OWASP categories down, six more to go. πŸ™‚

Leave a Reply

Your email address will not be published. Required fields are marked *