Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CVE-2022-2466 - Request Context not terminated with GraphQL #26748

Closed
yuxblank opened this issue Jul 15, 2022 · 28 comments · Fixed by #26777 or #26934
Closed

CVE-2022-2466 - Request Context not terminated with GraphQL #26748

yuxblank opened this issue Jul 15, 2022 · 28 comments · Fixed by #26777 or #26934
Labels
area/graphql kind/bug Something isn't working
Milestone

Comments

@yuxblank
Copy link

yuxblank commented Jul 15, 2022

Describe the bug

Upgrading from Quarkus 2.9.x to 2.10.x whenever i try to access RoutingContext request from a bean i get always the headers of the first request that have been made to the app.

This happens on both @GraphQLAPI endpoints and event in any CDI bean that consumes RoutingContext or HttpServerRequest.

Also by debugging, the CurrentVertxRequest has always stale headers.

Update: The request context was not terminated. The issue is not related to the Routing Context.

My dependencies are the following:

   <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-kotlin</artifactId>
    </dependency>
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-smallrye-graphql</artifactId>
    </dependency>
    <dependency>
      <groupId>io.smallrye</groupId>
      <artifactId>smallrye-jwt</artifactId>
    </dependency>
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-rest-client</artifactId>
    </dependency>
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-resteasy</artifactId>
    </dependency>
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-rest-client-jackson</artifactId>
    </dependency>
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-arc</artifactId>
    </dependency>
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-undertow</artifactId>
    </dependency>
    <dependency>
      <groupId>org.jetbrains.kotlin</groupId>
      <artifactId>kotlin-stdlib-jdk8</artifactId>
    </dependency>

CDI bean:

@ApplicationScoped
class JWTAwareContext @Inject constructor(
    val routingContext: RoutingContext
): Context {
    override fun getTenant(): String {
        return parseClaims()!!.getClaimValueAsString("realm")
    }

    override fun getUser(): String {
        return parseClaims()!!.getClaimValueAsString("email")
    }

    fun parseClaims(): JwtClaims? {
        val rawToken = this.routingContext.request().headers().get("Authorization")?.replace("Bearer ", "") // --> this will always stale in 2.10.x
        if (rawToken != null) {
            val jsonClaims = String(Base64.getUrlDecoder().decode(rawToken.split(".")[1]), StandardCharsets.UTF_8)
            return JwtClaims.parse(jsonClaims)
        }
        throw PassProNextForbiddenExceptionGQL(ApiError("Missing Token", ErrorCode.FORBIDDEN))
    }
}

Graphql endpoint

@GraphQLApi
class Test {
    @Inject lateinit var routingContext: RoutingContext


    @Query
    fun test(): String {
        return  "hello";
    }
}

With 2.9.x works correctly.

Expected behavior

Everytime a new Request is performed by a client, the Request headers should be inline with the actual HTTP Request

Actual behavior

With 2.10.x the first request headers became like cached value and any subsequent request headers will contain those instead of the actual headers

How to Reproduce?

  1. create an app with Quarkus 2.10.1 - 2.10.2 and the smallrye graphql extension
  2. create an endpoint or a bean injecting RoutingContext
  3. set some HTTP headers like Authorization, MyCustomHeader etc and send the http request
  4. print RoutingContext.request().headers
  5. set others HTTP headers or remove the previous and send the new http request
  6. the second request headers will contain first request data also if you did not send them
    7 ) switch to quarkus 2.9.x and will work as expected

Output of uname -a or ver

No response

Output of java -version

adopt-openjdk-11.0.8

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.10.1 - 2.10.2

Build tool (ie. output of mvnw --version or gradlew --version)

3.8.1

Additional information

No response

@yuxblank yuxblank added the kind/bug Something isn't working label Jul 15, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Jul 15, 2022

/cc @evanchooly

@yuxblank yuxblank changed the title RoutingContext request is stale RoutingContext request is stale (smallrye Graphql) Jul 15, 2022
@geoand
Copy link
Contributor

geoand commented Jul 15, 2022

cc @jmartisk @phillip-kruger

@phillip-kruger
Copy link
Member

Thanks, this is obviously an regression. Just out of interest, did you try and get the token with

@Inject
JsonWebToken jwt; 

As this might not have the issue

@yuxblank
Copy link
Author

yuxblank commented Jul 15, 2022

Thanks, this is obviously an regression. Just out of interest, did you try and get the token with

@Inject
JsonWebToken jwt; 

As this might not have the issue

No, its broken also JsonWebToken. Before making that example i was using JsonWebToken and when resolving the current request CTX and the Authorization header was stale.

For instance was like this:

 */
@ApplicationScoped
class JwtIdentityProvider : IdentityProvider<TokenAuthenticationRequest> {
    override fun getRequestType() = TokenAuthenticationRequest::class.java

    override fun authenticate(
        request: TokenAuthenticationRequest,
        context: AuthenticationRequestContext
    ): Uni<SecurityIdentity> {
        return try {
            val rawToken = request.token.token 
...

Also injecting JsonWebToken in GraphQLApi endpoints or any @RequestScoped bean had the same result

@phillip-kruger
Copy link
Member

@sberyozkin might be able to help ?

@markuseckstein
Copy link

Seems like a nasty security bug to me.

I set up a minimal repo for reproducing this behavior here:

https://github.com/markuseckstein/quarkus-issue-26748-reproducer

Tests pass with Quarkus 2.9.2.Final but fail since 2.10.0.Final

@phillip-kruger
Copy link
Member

@markuseckstein - thanks for the reproducer.

@gsmet
Copy link
Member

gsmet commented Jul 18, 2022

@phillip-kruger @jmartisk could you make this one a priority? I wonder if we should also get help from @manovotn , @mkouba or @Ladicek .

@phillip-kruger
Copy link
Member

I looked at this today and I did not find the issue. Still looking but any help is welcomed.

@Ladicek
Copy link
Contributor

Ladicek commented Jul 18, 2022

I can reproduce the issue with this simple class

@GraphQLApi
public class TestEndpoint {
    @Inject
    RoutingContext ctx;

    @Query("test")
    public String get() {
        return ctx.request().headers().get("X-Test");
    }
}

And these simple commands:

curl -X POST -H "X-Test: 123" -H "Content-Type: application/json" -d '{"query": "{ test }"}' http://localhost:8080/graphql
curl -X POST -H "X-Test: 456" -H "Content-Type: application/json" -d '{"query": "{ test }"}' http://localhost:8080/graphql
curl -X POST -H "X-Test: 789" -H "Content-Type: application/json" -d '{"query": "{ test }"}' http://localhost:8080/graphql

For some reason, the first 2 queries returns an expected result, but the 3rd does not.

I have also naively assumed that the same issue would happen in a simple JAX-RS resource, but it doesn't. Not sure why, but that suggests that the issue is a bit more complex than just scope misalignment that I originally expected.

@gsmet
Copy link
Member

gsmet commented Jul 18, 2022

I had a quick look and atm my guess is that this change is causing it: https://github.com/quarkusio/quarkus/pull/25194/files#diff-8f7eaa37c6a97d2c572deefa486e8762a7e962208beca6d2bb699b00a6327629R27

I'm building a patched version to check my theory.

@phillip-kruger
Copy link
Member

@Ladicek - yes I also ran the test with REST, and that worked, so it's GraphQL specific.

@phillip-kruger
Copy link
Member

@Ladicek @gsmet I think I found it. the endhandler that terminates the context. Doing a PR now. I'll add a test for this.

@gsmet
Copy link
Member

gsmet commented Jul 18, 2022

We have a fix that will get merged today and I will do an emergency release tomorrow.

Thanks for the report and thanks @markuseckstein for the isolated reproducer!

@yuxblank
Copy link
Author

We have a fix that will get merged today and I will do an emergency release tomorrow.

Thanks for the report and thanks @markuseckstein for the isolated reproducer!

thank you all for getting into so fast!

@quarkus-bot quarkus-bot bot added this to the 2.12 - main milestone Jul 18, 2022
@markuseckstein
Copy link

Great work! Thanks for fixing this so quickly!

@gsmet gsmet modified the milestones: 2.12 - main, 2.10.3.Final Jul 18, 2022
@cescoffier cescoffier changed the title RoutingContext request is stale (smallrye Graphql) REquest Context not terminated with GraphQL Jul 18, 2022
@cescoffier cescoffier changed the title REquest Context not terminated with GraphQL Request Context not terminated with GraphQL Jul 18, 2022
@gsmet
Copy link
Member

gsmet commented Jul 19, 2022

2.10.3.Final has been released. It fixes this issue. Note that 2.11.0.CR1 is also vulnerable. The fix will be included in 2.11.0.Final.

This issue is the object of CVE-2022-2466.

@gsmet gsmet changed the title Request Context not terminated with GraphQL CVE-2022-2466 - Request Context not terminated with GraphQL Jul 19, 2022
@yuxblank
Copy link
Author

yuxblank commented Jul 26, 2022

I'm sorry to say but the release 2.10.3.final did not fix the issue or not entirely.

I have tried to reproduce by modifying the reproducer ma de by @markuseckstein but i still didn't figured out of to make it breaking in the integration test.

The context it's getting terminated, but only the first time in my app, i try to explain:

  1. call the endpoint with a query or mutation with Authorization (or any other header) XXX and it's correct (currentManagedContextTerminationHandler is called after flush)
  2. call again the endpoint by removing headers (like authorization) and you still get the authorization header set previously in the older request ( currentManagedContextTerminationHandler has not been called after flush and the context is still active)

HTTP request

POST /graphql HTTP/1.1
Accept: application/json
Accept-Encoding: gzip, deflate, br
Accept-Language: it-IT,it;q=0.9,en-US;q=0.8,en;q=0.7
Connection: keep-alive
Content-Length: 892
Content-Type: application/json
Cookie: Idea-7e4a0a1b=2d5671e4-dabc-4e0f-931a-d199daba7f83
Host: localhost:8080
Origin: http://localhost:8080
Referer: http://localhost:8080/q/graphql-ui/?operationName=createAppStream&query=mutation%20createAppStream%20%7B%0A%20%20createAppStream(application%3A%22GesPro11%22)%2C%7B%0A%20%20%20%20streamingUrl%0A%20%20%7D%0A%7D%0A%0Aquery%20getAppstreamApps%20%7B%0A%20%20listAppStreamApps%20%7B%0A%20%20%20%20name%0A%20%20%20%20displayName%0A%20%20%20%20iconURL%0A%20%20%7D%0A%7D%0A%0Aquery%20VALIDATION%20%7B%0A%20%20getRule(ruleUuid%3A%22xx%22)%7B%0A%20%20%20%20code%0A%20%20%7D%0A%7D%0A%0Aquery%20NOT_FOUND%20%7B%0A%20%20%20%20%0A%20%20getRule(ruleUuid%3A%20%2266283770-ffa4-48a0-9d05-cfa712854563%22)%20%7B%0A%20%20%20%20code%0A%20%20%7D%0A%0A%7D%0A%0Amutation%20createRule%20%7B%0A%20%20createTariff(tariff%3A%7B%0A%20%20%20%20description%3A%22%22%2C%0A%20%20%20%20code%3A%22%22%2C%0A%20%20%20%20sector%3ALIFE%2C%0A%20%20%20%20uuid%3A%20%2266283770-ffa4-48a0-9d05-cfa712854563%22%2C%0A%20%20%20%20typeUuid%3A%2266283770-ffa4-48a0-9d05-cfa712854563%22%0A%20%20%7D)%20%7B%0A%20%20%20%20code%0A%20%20%7D%0A%7D%0A%0Aquery%20factor%20%7B%0A%20%20getFactor(factorUUID%3A%220ce8633d-059e-49ce-893c-d2e0ba51c328%22)%20%7B%0A%20%20%20%20code%0A%20%20%7D%0A%7D%0Aquery%20message%20%7B%0A%20%20getMessage(messageUUID%3A%220ce8633d-059e-49ce-893c-d2e0ba51c328%22)%20%7B%0A%20%20%20%20code%0A%20%20%7D%0A%7D
Sec-Fetch-Dest: empty
Sec-Fetch-Mode: cors
Sec-Fetch-Site: same-origin
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/103.0.0.0 Safari/537.36
sec-ch-ua: ".Not/A)Brand";v="99", "Google Chrome";v="103", "Chromium";v="103"
sec-ch-ua-mobile: ?0
sec-ch-ua-platform: "Windows"

Debug value of the routing context:

image

What is saw by debugging the SmallRyeGraphQLAbstractHandler class is that the first call effectively access the callback handler:

 this.currentManagedContextTerminationHandler = new Handler() {
            @Override
            public void handle(Object e) {
                currentManagedContext.terminate();
            }
        };

Subsequent requests do not.

Then in debug i've tried to terminate manually the context by calling terminate() in the handle function just at the beginning:

    public void handle(final RoutingContext ctx) {

        if (currentManagedContext.isActive()) {  --> break here and invoke currentManagedContext.terminate()
            handleWithIdentity(ctx);
        }

     ....
}

By doing so, the request get correctly terminated and headers are correctly reflected in the RoutingContext.request().

Also, invoking currentManagedContext.isActive() returns true also if the request is new.

Maybe has to do with terminating the context on the response flush?

The same behaviour is present both when there's an exception in between or when the response is successfull.

@gsmet
Copy link
Member

gsmet commented Jul 26, 2022

Even if you can't reproduce it in the ITs, it would help to have a reproducer with a simple app and curl commands to reproduce what's going wrong.
This problem is too sensitive for us to guess what you're doing.

@yuxblank
Copy link
Author

yuxblank commented Jul 26, 2022

@gsmet
Copy link
Member

gsmet commented Jul 26, 2022

@jmartisk @phillip-kruger @cescoffier could you have a look at what's going on?

@cescoffier
Copy link
Member

Are you using context propagation?

@yuxblank
Copy link
Author

yuxblank commented Jul 26, 2022

Are you using context propagation?

Yes.
Installed features: [cdi, kotlin, rest-client, rest-client-jackson, resteasy, servlet, smallrye-context-propagation, smallrye-graphql, vertx].

Using quarkus 2.9.x it works.

@phillip-kruger
Copy link
Member

I am investigating

@cescoffier
Copy link
Member

I've a fix. I communicated it to Phillip. As I don't know much about Graphql (unfortunately), Phillip will look and open the PR.

@phillip-kruger
Copy link
Member

Thanks @cescoffier ! PR to follow soon

@phillip-kruger
Copy link
Member

@gsmet - really sorry, this mean we probably need a release in the 2.10.x line ...

@yuxblank
Copy link
Author

Snapshots of the release are available to download? i can give it a run if it helps ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/graphql kind/bug Something isn't working
Projects
None yet
8 participants