Skip to content

Fix use-after-free in SearchApplicationIndexService buffer lifecycle#143134

Merged
ebarlas merged 6 commits intoelastic:mainfrom
ebarlas:use-after-free-in-search-app-index-service
Feb 26, 2026
Merged

Fix use-after-free in SearchApplicationIndexService buffer lifecycle#143134
ebarlas merged 6 commits intoelastic:mainfrom
ebarlas:use-after-free-in-search-app-index-service

Conversation

@ebarlas
Copy link
Copy Markdown
Contributor

@ebarlas ebarlas commented Feb 26, 2026

SearchApplicationIndexService.updateSearchApplication allocated a ReleasableBytesStreamOutput in a try-with-resources block, serialized JSON into it, then passed a BytesReference (a view over the buffer's pages) to an async client.index() call. The try-with-resources released the buffer's BigArrays pages before the async operation completed, leaving the IndexRequest source pointing at freed memory.

A second, subtler issue compounded the first: the inner try (XContentBuilder source = ...) closed the XContentBuilder, which cascaded through Jackson's JsonGenerator.close() to ReleasableBytesStreamOutput.close() via AUTO_CLOSE_TARGET (enabled by default). This released the buffer's pages even before buffer.bytes() was called.

The bug was latent until #142451 changed ReleasableBytesStreamOutput's default expectedSize from 0 to PAGE_SIZE_IN_BYTES. With expectedSize=0, BigArrays allocated a plain byte[] that was simply garbage-collected on release. With PAGE_SIZE_IN_BYTES, it allocates a recycled page that is immediately reusable after release, making the corruption observable: the replica node would read garbage bytes and throw a DocumentParsingException with "Illegal character (CTRL-CHAR, code 0)". This hit a TransportWriteAction assertion (failure instanceof MapperParsingException) that crashed the node, causing the Connection refused failures seen in CI.

The fix follows the pattern established by AsyncTaskIndexService:

  • Bind buffer release to the listener via ActionListener.runAfter(listener, buffer::close)
  • Call source.flush() instead of closing the XContentBuilder, to avoid transitively closing the buffer

@ebarlas ebarlas self-assigned this Feb 26, 2026
@ebarlas ebarlas added >bug :Security/Security Security issues without another label Team:Security Meta label for security team auto-backport Automatically create backport pull requests when merged v9.4.0 v9.3.2 v8.19.13 v9.2.7 labels Feb 26, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

elasticsearchmachine commented Feb 26, 2026

Pinging @elastic/es-security (Team:Security)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

elasticsearchmachine commented Feb 26, 2026

Hi @ebarlas, I've created a changelog YAML for you.

@ebarlas ebarlas mentioned this pull request Feb 26, 2026
Closed
DaveCTurner
DaveCTurner approved these changes Feb 26, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM nice catch, this would have been bad had the doc ever grown past 8kiB in the past.

.../src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java Outdated
clientWithOrigin.index(indexRequest, wrappedListener);
} catch (Exception e) {
listener.onFailure(e);
wrappedListener.onFailure(e);
Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a slight preference for ActionListener#run (makes it harder to forget to complete the wrapped listener here, which is important). But this is ok too.

@ebarlas ebarlas merged commit 755e020 into elastic:main Feb 26, 2026
41 checks passed
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

elasticsearchmachine commented Feb 26, 2026

💔 Backport failed

Status Branch Result
❌ 9.3 Commit could not be cherrypicked due to conflicts
❌ 8.19 Commit could not be cherrypicked due to conflicts
❌ 9.2 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 143134

PeteGillinElastic pushed a commit to PeteGillinElastic/elasticsearch that referenced this pull request Feb 27, 2026
@ebarlas @PeteGillinElastic
Fix search app buffer use-after-free (elastic#143134)
18f840e 
updateSearchApplication released a ReleasableBytesStreamOutput in
a try-with-resources block while an async client.index() call
still held a BytesReference view of its pages. 

The bug was latent until elastic#142451 switched the default buffer size
to PAGE_SIZE_IN_BYTES, causing BigArrays to recycle pages instead
of plain byte arrays. Freed pages were immediately reused,
corrupting the IndexRequest source on replica nodes.

Bind buffer release to the async listener via ActionListener.run
and flush instead of close, matching AsyncTaskIndexService.
@prwhelan prwhelan mentioned this pull request Feb 27, 2026
Merged
@ebarlas
Copy link
Copy Markdown
Contributor Author

ebarlas commented Feb 27, 2026

I'm going to manually apply the fix to the backport targets, since cherry pick failed.

ebarlas added a commit to ebarlas/elasticsearch that referenced this pull request Feb 27, 2026
@ebarlas
Fix search app buffer use-after-free (elastic#143134)
a00c527 
updateSearchApplication released a ReleasableBytesStreamOutput in
a try-with-resources block while an async client.index() call
still held a BytesReference view of its pages.

The bug was latent until elastic#142451 switched the default buffer size
to PAGE_SIZE_IN_BYTES, causing BigArrays to recycle pages instead
of plain byte arrays. Freed pages were immediately reused,
corrupting the IndexRequest source on replica nodes.

Bind buffer release to the async listener via ActionListener.run
and flush instead of close, matching AsyncTaskIndexService.
ebarlas added a commit to ebarlas/elasticsearch that referenced this pull request Feb 27, 2026
@ebarlas
Fix search app buffer use-after-free (elastic#143134)
9f45b0a 
updateSearchApplication released a ReleasableBytesStreamOutput in
a try-with-resources block while an async client.index() call
still held a BytesReference view of its pages.

The bug was latent until elastic#142451 switched the default buffer size
to PAGE_SIZE_IN_BYTES, causing BigArrays to recycle pages instead
of plain byte arrays. Freed pages were immediately reused,
corrupting the IndexRequest source on replica nodes.

Bind buffer release to the async listener via ActionListener.run
and flush instead of close, matching AsyncTaskIndexService.
ebarlas added a commit to ebarlas/elasticsearch that referenced this pull request Feb 27, 2026
@ebarlas
Fix search app buffer use-after-free (elastic#143134)
9da65be 
updateSearchApplication released a ReleasableBytesStreamOutput in
a try-with-resources block while an async client.index() call
still held a BytesReference view of its pages.

The bug was latent until elastic#142451 switched the default buffer size
to PAGE_SIZE_IN_BYTES, causing BigArrays to recycle pages instead
of plain byte arrays. Freed pages were immediately reused,
corrupting the IndexRequest source on replica nodes.

Bind buffer release to the async listener via ActionListener.run
and flush instead of close, matching AsyncTaskIndexService.
@ebarlas ebarlas mentioned this pull request Feb 27, 2026
Merged
ebarlas added a commit that referenced this pull request Mar 2, 2026
@ebarlas
Fix search app buffer use-after-free (#143134) (#143296)
ddf2136 
updateSearchApplication released a ReleasableBytesStreamOutput in
a try-with-resources block while an async client.index() call
still held a BytesReference view of its pages.

The bug was latent until #142451 switched the default buffer size
to PAGE_SIZE_IN_BYTES, causing BigArrays to recycle pages instead
of plain byte arrays. Freed pages were immediately reused,
corrupting the IndexRequest source on replica nodes.

Bind buffer release to the async listener via ActionListener.run
and flush instead of close, matching AsyncTaskIndexService.
ebarlas added a commit that referenced this pull request Mar 2, 2026
@ebarlas
Fix search app buffer use-after-free (#143134) (#143297)
4cb7af5 
updateSearchApplication released a ReleasableBytesStreamOutput in
a try-with-resources block while an async client.index() call
still held a BytesReference view of its pages.

The bug was latent until #142451 switched the default buffer size
to PAGE_SIZE_IN_BYTES, causing BigArrays to recycle pages instead
of plain byte arrays. Freed pages were immediately reused,
corrupting the IndexRequest source on replica nodes.

Bind buffer release to the async listener via ActionListener.run
and flush instead of close, matching AsyncTaskIndexService.
ebarlas added a commit that referenced this pull request Mar 2, 2026
@ebarlas
Fix search app buffer use-after-free (#143134) (#143298)
643f991 
updateSearchApplication released a ReleasableBytesStreamOutput in
a try-with-resources block while an async client.index() call
still held a BytesReference view of its pages.

The bug was latent until #142451 switched the default buffer size
to PAGE_SIZE_IN_BYTES, causing BigArrays to recycle pages instead
of plain byte arrays. Freed pages were immediately reused,
corrupting the IndexRequest source on replica nodes.

Bind buffer release to the async listener via ActionListener.run
and flush instead of close, matching AsyncTaskIndexService.
tballison pushed a commit to tballison/elasticsearch that referenced this pull request Mar 3, 2026
@ebarlas @tballison
Fix search app buffer use-after-free (elastic#143134)
ab6f9cd 
updateSearchApplication released a ReleasableBytesStreamOutput in
a try-with-resources block while an async client.index() call
still held a BytesReference view of its pages. 

The bug was latent until elastic#142451 switched the default buffer size
to PAGE_SIZE_IN_BYTES, causing BigArrays to recycle pages instead
of plain byte arrays. Freed pages were immediately reused,
corrupting the IndexRequest source on replica nodes.

Bind buffer release to the async listener via ActionListener.run
and flush instead of close, matching AsyncTaskIndexService.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DaveCTurner DaveCTurner DaveCTurner approved these changes

Assignees

@ebarlas ebarlas

Labels

auto-backport Automatically create backport pull requests when merged backport pending >bug :Security/Security Security issues without another label Team:Security Meta label for security team v8.19.13 v9.2.7 v9.3.2 v9.4.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ebarlas @elasticsearchmachine @DaveCTurner