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

SseUtil unexpectedly stores headers in Serialisers.EMPTY_MULTI_MAP #31559

Closed
cardil opened this issue Mar 2, 2023 · 5 comments · Fixed by #31560
Closed

SseUtil unexpectedly stores headers in Serialisers.EMPTY_MULTI_MAP #31559

cardil opened this issue Mar 2, 2023 · 5 comments · Fixed by #31560
Assignees
Labels
area/rest kind/bug Something isn't working
Milestone

Comments

@cardil
Copy link

cardil commented Mar 2, 2023

Describe the bug

The SseUtil class is using Serialisers.EMPTY_MULTI_MAP variable, here:

writer.writeTo(entity, entityClass, entityType, Serialisers.NO_ANNOTATION, mediaType,
Serialisers.EMPTY_MULTI_MAP, baos);

This leads to storing of subsequent headers in this map Serialisers.EMPTY_MULTI_MAP. This leads to uncapped memory consumption, use of the whole available heap (possible DDoS). Also, invocations have access to previously set headers, which is a privacy concern.

Expected behavior

Sending with SseUtil should be safe and not lead to data leaks or DDoS.

Actual behavior

Sending with SseUtil will lead to data leaks or DDoS.

How to Reproduce?

@GET
@RestStreamElementType(MediaType.APPLICATION_JSON)
Multi<CloudEvent> events() {
  return Multi.createFrom().iterable(events);
}

See also: cardil/openshift-knative-showcase@15e0042

Output of uname -a or ver

Linux thinkpad-t590 6.1.13-200.fc37.x86_64 #1 SMP PREEMPT_DYNAMIC Wed Feb 22 17:53:57 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "17.0.5" 2022-10-18
OpenJDK Runtime Environment Temurin-17.0.5+8 (build 17.0.5+8)
OpenJDK 64-Bit Server VM Temurin-17.0.5+8 (build 17.0.5+8, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.13.5.Final-redhat-00002

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

Apache Maven 3.8.7 (b89d5959fcde851dcb1c8946a785a163f14e1e29)

Additional information

/area resteasy-reactive

@cardil cardil added the kind/bug Something isn't working label Mar 2, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Mar 2, 2023

/cc @FroMage (resteasy-reactive), @Sgitario (resteasy-reactive), @stuartwdouglas (resteasy-reactive)

@geoand geoand self-assigned this Mar 2, 2023
geoand added a commit to geoand/quarkus that referenced this issue Mar 2, 2023
This could happen if a MessageBodyWriter writes the headers map
(which does not make sense for streaming responses, but it's
nevertheless allowed by the contract of the MessageBodyWriter)

Fixes: quarkusio#31559
@cardil
Copy link
Author

cardil commented Mar 2, 2023

I think the following should fix this issue:

MultivaluedHashMap<String, Object> empty = new MultivaluedHashMap<>(Serialisers.EMPTY_MULTI_MAP);
writer.writeTo(entity, entityClass, entityType, Serialisers.NO_ANNOTATION, mediaType, 
         empty, baos); 

@geoand
Copy link
Contributor

geoand commented Mar 2, 2023

I can't reproduce this, but clearly the code is wrong. #31560 fixes the issue

@cardil
Copy link
Author

cardil commented Mar 2, 2023

You need to have a writer that writes headers. Try cloud events library.

@geoand
Copy link
Contributor

geoand commented Mar 2, 2023

Right, I Imagined as much as explained in the description of the PR

geoand added a commit to geoand/quarkus that referenced this issue Mar 3, 2023
This could happen if a MessageBodyWriter writes the headers map
(which does not make sense for streaming responses, but it's
nevertheless allowed by the contract of the MessageBodyWriter)

Fixes: quarkusio#31559
geoand added a commit that referenced this issue Mar 3, 2023
Prevent SSE writing from potentially causing accumulation of headers
@quarkus-bot quarkus-bot bot added this to the 3.0 - main milestone Mar 3, 2023
@gsmet gsmet modified the milestones: 3.0.0.Alpha5, 2.16.5.Final Mar 15, 2023
gsmet pushed a commit to gsmet/quarkus that referenced this issue Mar 16, 2023
This could happen if a MessageBodyWriter writes the headers map
(which does not make sense for streaming responses, but it's
nevertheless allowed by the contract of the MessageBodyWriter)

Fixes: quarkusio#31559
(cherry picked from commit 4cb3797)
gsmet pushed a commit to gsmet/quarkus that referenced this issue Mar 16, 2023
This could happen if a MessageBodyWriter writes the headers map
(which does not make sense for streaming responses, but it's
nevertheless allowed by the contract of the MessageBodyWriter)

Fixes: quarkusio#31559
(cherry picked from commit 4cb3797)
gsmet pushed a commit to gsmet/quarkus that referenced this issue Mar 16, 2023
This could happen if a MessageBodyWriter writes the headers map
(which does not make sense for streaming responses, but it's
nevertheless allowed by the contract of the MessageBodyWriter)

Fixes: quarkusio#31559
(cherry picked from commit 4cb3797)
gsmet pushed a commit to gsmet/quarkus that referenced this issue Mar 20, 2023
This could happen if a MessageBodyWriter writes the headers map
(which does not make sense for streaming responses, but it's
nevertheless allowed by the contract of the MessageBodyWriter)

Fixes: quarkusio#31559
(cherry picked from commit 4cb3797)
benkard added a commit to benkard/mulkcms2 that referenced this issue Apr 4, 2023
This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [flow-bin](https://github.com/flowtype/flow-bin) ([changelog](https://github.com/facebook/flow/blob/master/Changelog.md)) | devDependencies | minor | [`^0.201.0` -> `^0.203.0`](https://renovatebot.com/diffs/npm/flow-bin/0.201.0/0.203.1) |
| [com.rometools:rome](http://rometools.com) ([source](https://github.com/rometools/rome)) | compile | minor | `2.0.0` -> `2.1.0` |
| [org.postgresql:postgresql](https://jdbc.postgresql.org) ([source](https://github.com/pgjdbc/pgjdbc)) | build | minor | `42.5.4` -> `42.6.0` |
| [com.diffplug.spotless:spotless-maven-plugin](https://github.com/diffplug/spotless) | build | minor | `2.34.0` -> `2.35.0` |
| [org.apache.maven.plugins:maven-resources-plugin](https://maven.apache.org/plugins/) | build | patch | `3.3.0` -> `3.3.1` |
| [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | patch | `2.16.4.Final` -> `2.16.6.Final` |
| [io.quarkus:quarkus-universe-bom](https://github.com/quarkusio/quarkus-platform) | import | patch | `2.16.4.Final` -> `2.16.6.Final` |

---

### Release Notes

<details>
<summary>flowtype/flow-bin</summary>

### [`v0.203.1`](flow/flow-bin@0c16b26...5e0645d)

[Compare Source](flow/flow-bin@0c16b26...5e0645d)

### [`v0.203.0`](flow/flow-bin@861f798...0c16b26)

[Compare Source](flow/flow-bin@861f798...0c16b26)

### [`v0.202.1`](flow/flow-bin@2b48bba...861f798)

[Compare Source](flow/flow-bin@2b48bba...861f798)

### [`v0.202.0`](flow/flow-bin@86aea9c...2b48bba)

[Compare Source](flow/flow-bin@86aea9c...2b48bba)

</details>

<details>
<summary>rometools/rome</summary>

### [`v2.1.0`](https://github.com/rometools/rome/releases/tag/2.1.0)

[Compare Source](rometools/rome@2.0.0...2.1.0)

<!-- Release notes generated using configuration in .github/release.yml at 2.1.0 -->

#### What's Changed

##### ⭐ New Features

-   Downgrade Java from version 11 to 8 by [@&#8203;PatrickGotthard](https://github.com/PatrickGotthard) in rometools/rome#642
-   Add support for GraalVM native images by [@&#8203;artembilan](https://github.com/artembilan) in rometools/rome#636

##### 🔨 Dependency Upgrades

-   Bump maven-compiler-plugin from 3.10.1 to 3.11.0 by [@&#8203;dependabot](https://github.com/dependabot) in rometools/rome#635

##### 🧹 Cleanup

-   Remove unused config files by [@&#8203;PatrickGotthard](https://github.com/PatrickGotthard) in rometools/rome#632
-   Polish GitHub workflows by [@&#8203;PatrickGotthard](https://github.com/PatrickGotthard) in rometools/rome#633
-   Polish code by [@&#8203;antoniosanct](https://github.com/antoniosanct) in rometools/rome#631

##### ✔ Other Changes

-   Update configuration for automatically generated release notes by [@&#8203;PatrickGotthard](https://github.com/PatrickGotthard) in rometools/rome#634

#### New Contributors

-   [@&#8203;artembilan](https://github.com/artembilan) made their first contribution in rometools/rome#636

**Full Changelog**: rometools/rome@2.0.0...2.1.0

</details>

<details>
<summary>pgjdbc/pgjdbc</summary>

### [`v42.6.0`](https://github.com/pgjdbc/pgjdbc/blob/HEAD/CHANGELOG.md#&#8203;4260-2023-03-17-153434--0400)

##### Changed

fix: use PhantomReferences instead of `Obejct.finalize()` to track Connection leaks [MR #&#8203;2847](pgjdbc/pgjdbc#2847)

    The change replaces all uses of Object.finalize with PhantomReferences.
    The leaked resources (Connections) are tracked in a helper thread that is active as long as
    there are connections in use. By default, the thread keeps running for 30 seconds after all
    the connections are released. The timeout is set with pgjdbc.config.cleanup.thread.ttl system property.

refactor:(loom) replace the usages of synchronized with ReentrantLock [MR #&#8203;2635](pgjdbc/pgjdbc#2635)
Fixes [Issue #&#8203;1951](pgjdbc/pgjdbc#1951)

</details>

<details>
<summary>diffplug/spotless</summary>

### [`v2.35.0`](https://github.com/diffplug/spotless/blob/HEAD/CHANGES.md#&#8203;2350---2023-02-10)

##### Added

-   CleanThat Java Refactorer. ([#&#8203;1560](diffplug/spotless#1560))
-   Introduce `LazyArgLogger` to allow for lazy evaluation of log messages in slf4j logging. ([#&#8203;1565](diffplug/spotless#1565))

##### Fixed

-   Allow multiple instances of the same npm-based formatter to be used by separating their `node_modules` directories. ([#&#8203;1565](diffplug/spotless#1565))
-   `ktfmt` default style uses correct continuation indent. ([#&#8203;1562](diffplug/spotless#1562))

##### Changes

-   Bump default `ktfmt` version to latest `0.42` -> `0.43` ([#&#8203;1561](diffplug/spotless#1561))
-   Bump default `jackson` version to latest `2.14.1` -> `2.14.2` ([#&#8203;1536](diffplug/spotless#1536))

</details>

<details>
<summary>quarkusio/quarkus</summary>

### [`v2.16.6.Final`](https://github.com/quarkusio/quarkus/releases/tag/2.16.6.Final)

[Compare Source](quarkusio/quarkus@2.16.5.Final...2.16.6.Final)

##### Complete changelog

-   [#&#8203;32319](quarkusio/quarkus#32319) - \[2.16] Revert io.netty.noUnsafe change
-   [#&#8203;32302](quarkusio/quarkus#32302) - Qute - fix validation of expressions with the "cdi" namespace
-   [#&#8203;32253](quarkusio/quarkus#32253) - (2.16) Upgrade to graphql-java 19.4
-   [#&#8203;32223](quarkusio/quarkus#32223) - (2.16) Upgrade wildfly-elytron to 1.20.3.Final
-   [#&#8203;32110](quarkusio/quarkus#32110) - Prevent splitting of cookie header values when using AWS Lambda
-   [#&#8203;32107](quarkusio/quarkus#32107) - Fix Podman detection on Windows
-   [#&#8203;32106](quarkusio/quarkus#32106) - Native building with container: Podman not detected on Windows
-   [#&#8203;32093](quarkusio/quarkus#32093) - Re-use current ApplicationModel for JaCoCo reports when testing Gradle projects
-   [#&#8203;32090](quarkusio/quarkus#32090) - K8s moved its registry
-   [#&#8203;32088](quarkusio/quarkus#32088) - Remove the session cookie if ID token verification failed
-   [#&#8203;32082](quarkusio/quarkus#32082) - Add missing quote in Hibernate Reactive with Panache guide
-   [#&#8203;32079](quarkusio/quarkus#32079) - Quarkus JaCoCo extension fails to start Gradle daemon
-   [#&#8203;32058](quarkusio/quarkus#32058) - Allow use of null in REST Client request body
-   [#&#8203;32047](quarkusio/quarkus#32047) - rest client reactive throws npe on null request body
-   [#&#8203;32041](quarkusio/quarkus#32041) - K8s is moving it's images
-   [#&#8203;32037](quarkusio/quarkus#32037) - Set-Cookie Header is Split when using OIDC together with AWS Lambda
-   [#&#8203;32015](quarkusio/quarkus#32015) - Support repeatable Incomings annotation for reactive messaging
-   [#&#8203;32002](quarkusio/quarkus#32002) - Quarkus: Kafka Event Processor with 2 `@incoming` annotations throws Null Pointer SRMSG00212
-   [#&#8203;31984](quarkusio/quarkus#31984) - Only substitute OctetKeyPair\* classes when on the classpath
-   [#&#8203;31978](quarkusio/quarkus#31978) - Remove quarkus.hibernate-orm.database.generation=drop-and-create from Hibernate ORM codestart
-   [#&#8203;31930](quarkusio/quarkus#31930) - Native build fails for JWT
-   [#&#8203;31893](quarkusio/quarkus#31893) - Docker or Podman required for tests since 3.0.0.Alpha6
-   [#&#8203;31857](quarkusio/quarkus#31857) - Container runtime detection cached in sys prop, container-docker extension
-   [#&#8203;31811](quarkusio/quarkus#31811) - Check the expiry date for inactive OIDC tokens
-   [#&#8203;31717](quarkusio/quarkus#31717) - Quarkus OIDC Session Cookie not deleted in case of 401 unauthorized
-   [#&#8203;31714](quarkusio/quarkus#31714) - OIDC token refresh fails with 401, if user info is used and not available in the cache (anymore)
-   [#&#8203;31662](quarkusio/quarkus#31662) - Warning when docker is not running
-   [#&#8203;31525](quarkusio/quarkus#31525) - Bump Keycloak version to 21.0.1
-   [#&#8203;31490](quarkusio/quarkus#31490) - Enable Podman and Docker Windows quarkus-container-image-docker testing
-   [#&#8203;31307](quarkusio/quarkus#31307) - Native Build on Windows has incorrect resource slashes
-   [#&#8203;30383](quarkusio/quarkus#30383) - Create a new base classloader including parent-first test scoped dependencies when bootstrapping for CT

### [`v2.16.5.Final`](https://github.com/quarkusio/quarkus/releases/tag/2.16.5.Final)

[Compare Source](quarkusio/quarkus@2.16.4.Final...2.16.5.Final)

##### Complete changelog

-   [#&#8203;31959](quarkusio/quarkus#31959) - New home for Narayana LRA coordinator Docker images
-   [#&#8203;31931](quarkusio/quarkus#31931) - Support raw collections in RESTEasy Reactive server and client
-   [#&#8203;31922](quarkusio/quarkus#31922) - Add more lenient Liquibase ZipPathHandler to work around includeAll not working in prod mode
-   [#&#8203;31904](quarkusio/quarkus#31904) - \[2.16] Upgrade SmallRye GraphQL to 1.9.4
-   [#&#8203;31894](quarkusio/quarkus#31894) - Supply missing extension metadata for reactive keycloak client
-   [#&#8203;31891](quarkusio/quarkus#31891) - Fix truststore REST Client config when password is not set
-   [#&#8203;31867](quarkusio/quarkus#31867) - Qute type-safe fragments - fix validation for loop metadata and globals
-   [#&#8203;31866](quarkusio/quarkus#31866) -  The behavior of the `@RestHeader` annotation is different from the `@HeaderParam` annotation when the parameter is of type List
-   [#&#8203;31864](quarkusio/quarkus#31864) - Fix incorrect generic type passed to MessageBodyWriter#writeTo
-   [#&#8203;31818](quarkusio/quarkus#31818) - Jackson JAX-RS YAML Provider for Resteasy Reactive
-   [#&#8203;31804](quarkusio/quarkus#31804) - \[2.16] A test to make sure non-existing modules are ignored during workspace discovery
-   [#&#8203;31793](quarkusio/quarkus#31793) - \[2.16] Fix NPE loading workspace modules
-   [#&#8203;31770](quarkusio/quarkus#31770) - Fix native compilation when using quarkus-jdbc-oracle with elasticsearch-java
-   [#&#8203;31769](quarkusio/quarkus#31769) - Capability added for quarkus-rest-client-reactive-jackson
-   [#&#8203;31756](quarkusio/quarkus#31756) - quarkus-rest-client-reactive-jackson doesn't provide capabilities
-   [#&#8203;31728](quarkusio/quarkus#31728) - Register additional cache implementations for reflection
-   [#&#8203;31718](quarkusio/quarkus#31718) - Properly close metadata file in integration tests
-   [#&#8203;31713](quarkusio/quarkus#31713) - "Too many open files" When test native image.
-   [#&#8203;31712](quarkusio/quarkus#31712) - Make request scoped beans work properly in ReaderInterceptors
-   [#&#8203;31705](quarkusio/quarkus#31705) - Remove all dev services for kubernetes dependencies from kubernetes-client-internal
-   [#&#8203;31692](quarkusio/quarkus#31692) - RequestScoped context not active when using a ReaderInterceptor with large HTTP requests
-   [#&#8203;31688](quarkusio/quarkus#31688) - Suppress config changed warning for quarkus.test.arg-line
-   [#&#8203;31643](quarkusio/quarkus#31643) - Fix iterator issue when executing a zrange with score on a missing key
-   [#&#8203;31626](quarkusio/quarkus#31626) - quarkus.test.arg-line has become a built-time fixed property in 2.16.4
-   [#&#8203;31624](quarkusio/quarkus#31624) - native compilation : quarkus-jdbc-oracle with elasticsearch-java strange behaviour
-   [#&#8203;31617](quarkusio/quarkus#31617) - Bump Stork version 1.4.2
-   [#&#8203;31579](quarkusio/quarkus#31579) - Reinitialize sun.security.pkcs11.P11Util at runtime
-   [#&#8203;31560](quarkusio/quarkus#31560) - Prevent SSE writing from potentially causing accumulation of headers
-   [#&#8203;31559](quarkusio/quarkus#31559) - `SseUtil` unexpectedly stores headers in `Serialisers.EMPTY_MULTI_MAP`
-   [#&#8203;31551](quarkusio/quarkus#31551) - Scheduler - detect scheduled methods of the same name on a class
-   [#&#8203;31547](quarkusio/quarkus#31547) - Scheduler - it's possible to declare two scheduled methods of the same name on the same class
-   [#&#8203;31545](quarkusio/quarkus#31545) - Append System.lineSeparator() to config error messages
-   [#&#8203;31536](quarkusio/quarkus#31536) - Missing newline characters in config error message
-   [#&#8203;31532](quarkusio/quarkus#31532) - Interpret negative/zero body-limit as infinite when logging REST Client request body
-   [#&#8203;31523](quarkusio/quarkus#31523) - Request rejected by CORS for fonts in dev UI when `quarkus.http.cors=true` is set
-   [#&#8203;31496](quarkusio/quarkus#31496) - Filter out RESTEasy related warning in ProviderConfigInjectionWarningsTest
-   [#&#8203;31482](quarkusio/quarkus#31482) - Remove incorrect default value for keepAliveEnabled
-   [#&#8203;31440](quarkusio/quarkus#31440) - Several quarkus integration tests fail to compile to native with latest GraalVM master
-   [#&#8203;31384](quarkusio/quarkus#31384) - Ignore required documentation for `@ConfigMapping` default methods
-   [#&#8203;30757](quarkusio/quarkus#30757) - Allow same origin CORS requests without 3rd party origins being configured
-   [#&#8203;30744](quarkusio/quarkus#30744) - \[Quarkus Native] ClassNotFoundException: com.github.benmanes.caffeine.cache.SSSW
-   [#&#8203;30698](quarkusio/quarkus#30698) - CORS Request same origin ignored if no other origin set

</details>

<details>
<summary>quarkusio/quarkus-platform</summary>

### [`v2.16.6.Final`](quarkusio/quarkus-platform@2.16.5.Final...2.16.6.Final)

[Compare Source](quarkusio/quarkus-platform@2.16.5.Final...2.16.6.Final)

### [`v2.16.5.Final`](quarkusio/quarkus-platform@2.16.4.Final...2.16.5.Final)

[Compare Source](quarkusio/quarkus-platform@2.16.4.Final...2.16.5.Final)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

👻 **Immortal**: This MR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
@gsmet gsmet modified the milestones: 2.16.5.Final, 2.13.8.Final May 9, 2023
gsmet pushed a commit to gsmet/quarkus that referenced this issue May 9, 2023
This could happen if a MessageBodyWriter writes the headers map
(which does not make sense for streaming responses, but it's
nevertheless allowed by the contract of the MessageBodyWriter)

Fixes: quarkusio#31559
(cherry picked from commit 4cb3797)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants