OWASP A3:2017 – Sensitive Data Exposure

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. So, without further ado, let’s get started.

Table of Contents

Challenge 1 – Insufficiently Protected Credentials

Locating Vulnerability

This challenge is simple, we just need to find insecurely stored passwords or cryptographic keys in application source code. If we look into the CryptoService.php file marked with ⚠️, we’ll see that on line 10, the cryptographic key is hard-coded into the application source code via the $cryptoKey variable. If an attacker is able to compromise the application, then he can get access to that key and in turn decrypt all user details(including credentials) of users doing registration on the web application.

<?php

namespace App\services;

use App\services\logger\Logger;

class CryptoService
{
    private static string $cipherAlgorithm = 'aes-128-gcm';
    private static string $cryptoKey = 'ab86d144e3f080b61c7c2e43';
    private static $result;
    private static int $failed;

Identifying solution

OWASP has laid out some basic rules to follow while storing Cryptographic keys which are:-

  • Do not hard-code keys into the application source code.
  • Do not check keys into version control systems.
  • Protect the configuration files containing the keys with restrictive permissions.
  • Avoid storing keys in environment variables, as these can be accidentally exposed through functions such as phpinfo() or through the /proc/self/environ file.

In the revised application source code only one solution moves the secret key outside of the application and stores it in a config file. While this is not the most secure way of storing cryptographic keys, it can prevent an attacker from reading the secret key via other vulnerabilities in the application. The desired solution of the challenge imports the crypto_key variable from a config file stored outside the application folder via getConfig() function of Config class.

class CryptoService
{
    private static string $cipherAlgorithm = 'aes-128-gcm';
    private static $result;
    private static int $failed;

    public static function encrypt(string $data)
    {
        if (\in_array(self::$cipherAlgorithm, openssl_get_cipher_methods(), true))
        {
            $ivLegth = openssl_cipher_iv_length(self::$cipherAlgorithm);
            $randomBytes = openssl_random_pseudo_bytes($ivLegth);

            if ($randomBytes !== false) {
                self::$result = openssl_encrypt(
                    $data,
                    self::$cipherAlgorithm,
                    Config::getConfig()['crypto_key'],
                    $options=0,
                    $randomBytes
                );
            }

The config file can be accessed via the Config.php file which defines the location of the config file. The following is source code of Config.php which shows that the config file is stored outside the app.

<?php

namespace App\config;

class Config
{
    /**
     * @return array
     */
    public static function getConfig(): array
    {
        $config = __DIR__ . '/../../config.ini';
        return parse_ini_file($config, true);
    }
}

Why other solutions are incorrect?

  • Config.ini should not be included in the source code and .gitignore must not be pointing to it
  • The crypto key should not be included in the application source code like login.php and register.php.
  • Crypto key should not be shown in comments. Never use comments for such things!!

Challenge 2 – Weak Algorithm Use

Locating Vulnerability

As the challenge name suggest we must be looking for weak algorithm usage in to application source code. This challenge is easy although a bit trickier to find correct weak algorithm usage. You’ve to reference every available algorithm used in the source code with official PHP documentation to find the correct vulnerability. As such, the CryptoService.php file uses the now deprecated PHP mcrypt_encrypt() function to encrypt user data. The following source code shows the vulnerable function.

class CryptoService
{
    public static function encrypt(string $data)
    {
        $encrypt = serialize($data);
        $iv = mcrypt_create_iv(
            mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_CBC),
            MCRYPT_DEV_RANDOM
        );

        if ($iv === false) {
            self::encrypt($data);
        }

        $key = pack('H*', Config::getConfig()['crypto_key']);
        $mac = hash_hmac('sha1', $encrypt, substr(bin2hex($key), -32));
        $passcrypt = mcrypt_encrypt(
            MCRYPT_RIJNDAEL_256,
            $key,
            $encrypt.$mac,
            MCRYPT_MODE_CBC, $iv
        );
        $encoded = base64_encode($passcrypt).'|'.base64_encode($iv);

        Logger::info('Data successfully encrypted');

        return $encoded;
    }

Usage of weak data encryption algorithm can result in sensitive data exposure to malicious users. A hacker can mount further attacks on the webapp with the sensitive data and can potentially gain access to the underlying server. Now, I’m not going to try to reinvent the wheel and bore the reader with technical details as to why the above code is vulnerable and would rather want them to read this excellent guide and have fun learning about the vulnerability in the above code. 🙂

Identifying Solution

If you went through the excellent guide I suggested above, then you already know what must be the solution for fixing the vulnerable code. As such, only one solution code implements the openssl_encrypt() function. Also, the cipher algorithm used in the solution is “aes-128-gcm” which, last I checked, is still considered a strong encryption algorithm. Now, I’ve decided not to steal the fun for myself and anybody else referring this guide in the future and for the sake of brevity, I’m not listing the secure source code here as it is already given in the guide I suggested above. 🙂

Challenge 3 – Plain Text Storage of Passwords

Locating Vulnerability

As the challenge is about looking for storage of plain text passwords into the database, we should be looking for functions that does not hash the password using a secure algorithm before storing it into the database. As such, the register() function of RegistrationService.php is vulnerable as it stores the user passwords directly as plain text without hashing them.

    public function register(Request $request): bool
    {
        foreach ($request as $item) {
            if (empty($item)) {
                throw new InvalidArgumentException('Registration failed');
            }
        }

        $json = json_decode($request, true, 512, JSON_THROW_ON_ERROR);

        $sanitizedData = $this->checkDataService->sanitize($json);

        $text = $this->checkDataService->isValidData([
            $sanitizedData['name'],
            $sanitizedData['surname'],
        ]);

        $phone = $this->checkDataService->isValidPhone($sanitizedData['phone']);
        $email = $this->checkDataService->isValidEmail($sanitizedData['email']);
        $password = $this->checkDataService->isValidPassword($sanitizedData['password']);

        $encryptedPhone = CryptoService::encrypt($sanitizedData['phone']);
        if ($phone && !empty($sanitizedData['phone']) && $encryptedPhone !== null) {
            $this->userDTO->phone = $encryptedPhone;
        }

        $encryptedEmail = CryptoService::encrypt($sanitizedData['email']);
        if ($email && !empty($sanitizedData['email']) && $encryptedEmail !== null) {
            $this->userDTO->email = $encryptedEmail;
        }

        if ($password && !empty($sanitizedData['password'])) {
            $this->userDTO->password = $sanitizedData['password'];
        }

On line 60, you can see that when a user registers the password, although sanitized, is not hashed in anyway before it is stored in the database. The other credentials of the user, i.e., Email and Phone, are encrypted using the encrypt() function of CryptoService class before storing them in the database.

Identifying Solution

As we already know that the password needs to be hashed before being stored in the database, now all we need to do to find the right solution is look for a solution that implements a strong algorithm recommended by PHP and OWASP. As such only one solution creates a hash of the password using a strong algorithm, i.e., Argon2id which is the algorithm recommended by OWASP.

 $passwordHash = Hash::make($sanitizedData['password']);
        if ($password && !empty($sanitizedData['password']) && $passwordHash !== null) {
            $this->userDTO->password = $passwordHash;
}

Here, the make() function of Hash class is used to create a secure hash of the password. If we look into Hash.php in the /application/services/generators/ folder we’ll see that an secure password hash is created using the PHP password_hash() function which uses the Argon2id algorithm. The other solutions are incorrect because they use insecure PHP functions like sha1, md5 and base64_encode() to secure the password.

<?php

namespace App\services\generators;

class Hash
{
    /**
     * @param string $data
     * @return false|string
     */
    public static function make(string $data)
    {
        $hash = password_hash($data, PASSWORD_ARGON2ID, ['memory_cost' => 2048, 'time_cost' => 4, 'threads' => 3]);

        if (!empty($hash)) {
            return $hash;
        }

        return false;
    }
}

Conclusion

Three OWASP categories down, seven more to go. 🙂

Leave a Reply

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