[Feature Request]: Discovery supply chain attacks

[SamMousa]

Description

This is not a FR, but it's also not a bug report...

Really cool to see the work on this. I've been thinking on risks of exploiting the supply chain when discovery is used. Note: I haven't coded anything in Tempest, so this is based on reading docs / watching your videos.

Supply chain attacks happen more and more but there are some impediments:

1. Composer will no longer execute plugins by default: https://getcomposer.org/doc/06-config.md#allow-plugins
2. Code from a dependency is not callable by an attacker directly, they need to find some vulnerable way your code is calling their planted code.

Discovery essentially solves part 2. I could now in any library, possibly even in a test file, annotate something with a #Route and make it callable from the web.

Wondering about your thoughts on this!

Benefits

N/A

[brendt]

You're right that there's a possible security risk here, although the same could be done in Laravel or Symfony, no? It's possible to register malicious routes within vendor packages in both frameworks. If a user blindly requires a package it could in theory contain malicious code.

I wonder how Laravel or Symfony address this potential security risk.

[SamMousa]

I'm not a Laravel fan, not am i familiar with how they scan vendor packages. Seems like risky business to me! *-)

Symfony docs suggest some explicit configuration is required: https://symfony.com/doc/current/routing.html#creating-routes-as-attributes

Laravel also uses a route file, making it more of an explicit action: https://laravel.com/docs/11.x/routing#the-default-route-files

If a user blindly requires a package it could in theory contain malicious code.

This is true, but it's not really feasible to check all code (transitive dependencies). Normally a package requiring package A could just validate that the code it calls from A is safe, now it needs to verify everything in the repo is safe.

[brendt]

I believe the same is possible in both Symfony and Laravel though: a package could access the DI container, thus is can access the router, thus it can directly register routes in the router — even without using attributes.

I'm not sure how it'd work in Symfony, but in Laravel there even are helper functions to do that: https://laravel.com/docs/11.x/packages#routes

I don't think you can ever be sure a package doesn't do anything malicious except for when you manually review the while codebase.

[SamMousa]

I believe the same is possible in both Symfony and Laravel though: a package could access the DI container, thus is can access the router, thus it can directly register routes in the router — even without using attributes.

But it still needs to call those functions actively.

With any CVE while a package might be vulnerable, not all applications using the package will be vulnerable. Even if there's malicious intent, it'd still need to be part of every code path into the library, making it easier to detect.

But if I update some test file to have a #Route() attribute it is less likely to be spotted while I can still be 100% certain I can trigger the vulnerability in a trivial way in 100% of the cases.

I think a clean way forward would be similar to composers' approach: when first detecting routes or other comparable things inside a dependency, ask if you want to trust them. This prevents your simple utility library that just computes a+b from installing an exploit in your app.

[aidan-casey]

Hey, @SamMousa! Thanks for opening this issue. It is certainly an important conversation to have as we consider this feature. I know Brent and I spent quite some time discussing the implications early on.

I think the nearest "equivalent" to what discovery is doing (and indeed the reason discovery works) is the autoloading that composer provides. If I can get any PHP file loaded, that gives me the ability to hijack your application. This is a risk with any PHP application and something tools like composer audit and Snyk are trying to help solve. At the end of the day it is our responsibility as developers, daunting as it may be, to ensure we are installing safe and reputable dependencies.

Because of this, I really don't think we're dealing with any level of risk that is different than that of Symfony or Laravel. That said, maybe this does get spun as a feature request for somewhere in the future where you can enter a Tempest audit CLI command to do deep auditing of discovery or the enabling of a security mode, which requires whitelisting discovered classes somewhere.