Skip to content

re-serialize JSON metadata written into the ES and OpenSearch _bulk body - #3603

Open
mrdevel2002-ui wants to merge 1 commit into
apache:mainfrom
mrdevel2002-ui:opensearch-es-bulk-json-injection
Open

mrdevel2002-ui wants to merge 1 commit into
apache:mainfrom
mrdevel2002-ui:opensearch-es-bulk-json-injection

Conversation

@mrdevel2002-ui

Copy link
Copy Markdown
Contributor

The OpenSearch and Elasticsearch emitters write JSON-typed metadata values (the reserved tk:chunks vector/locator field in particular) into the NDJSON _bulk request body with JsonGenerator.writeRawValue(vals[0]). That value is attacker influenced: a FetchEmitTuple's metadata block submitted to tika-server /pipes or /async reaches the emitter unescaped, because Metadata.reconstruct writes the reserved key through setTrusted and EmitHandler.injectUserMetadata copies it onto the emitted document, and tk:chunks is declared ValueType.JSON so isJsonField routes it to the raw-write path.

OpenSearchClient.isValidJson gated the value with ObjectMapper.readTree, which stops at the first JSON value and silently ignores anything after it. A tk:chunks string of [1]} followed by newline-separated bulk actions therefore passes the check, and writeRawValue emits it verbatim, so the bulk body becomes:

{"index":{"_id":"k"}}
{...,"tk:chunks":[1]}
{"delete":{"_index":"victim","_id":"1"}}
{"index":{"_index":"junk"}}
{"pad":[1],...}

The } closes the source document early and the embedded newlines add a delete and an index action; the injected actions can name any _index, so one tuple reaches indices the operator never configured this emitter for (bounded only by the cluster's rest.action.multi.allow_explicit_index). ESClient.isValidJson was already strict about trailing content (testTrailingContentRejected), so Elasticsearch is not open to that clean injection, but both clients still passed the value through raw, so an embedded newline in an otherwise valid value splits the NDJSON line and corrupts or injects actions there too.

The fix brings OpenSearch's isValidJson in line with the strict Elasticsearch one and, in both clients, re-serializes the parsed node (readTree(vals[0]).toString()) instead of writing the original string. Re-serialization drops anything after the first value and strips insignificant whitespace, so a crafted value can no longer add or split _bulk action lines; a legitimate chunks array still indexes natively, since vectors are base64 text and locators round-trip unchanged. The fix lives in the emitter because that is the one place the untrusted value crosses into the bulk protocol. testChunksCannotInjectBulkActions (OpenSearch) and testChunksWithNewlinesStayOnOneLine (ES) fail on main and pass with the change. Full tika-pipes-opensearch and tika-pipes-es builds (tests, checkstyle, spotless, forbidden-apis, apache-rat) are green on JDK 21.

  • open issue on the Tika issue tracker: not filed yet, happy to open one or have it linked
  • issue ID (TIKA-XXXX) in the PR title and commit message: pending the issue above
  • commits are squashed into a single one
  • Tika builds and unit tests pass with ./mvnw clean install (affected modules)
  • no conflicts with the recent main branch
  • new downstream module added to tika-bom/pom.xml: not applicable

The OpenSearch and Elasticsearch emitters wrote JSON-typed metadata
values such as the reserved tk:chunks key into the NDJSON _bulk body with
writeRawValue(vals[0]). That value is attacker influenced: a tuple's
metadata block submitted to /pipes or /async reaches the emitter
unescaped. OpenSearch's isValidJson used readTree, which ignores content
after the first value, so a value like [1]} followed by newline-separated
bulk actions closed the source document early and injected arbitrary
delete/index actions against any index the credentials allow. ES was
already strict about trailing content but still passed the value raw, so
an embedded newline split the NDJSON line there too. Make OpenSearch
strict like ES, and in both clients re-serialize the parsed node so
trailing content is dropped and insignificant whitespace is stripped; a
valid chunks array still indexes natively.

Generated-by: Claude Code (claude-opus-4-8)
@tballison

Copy link
Copy Markdown
Contributor

Can you have your bot tone down the description on this issue and going forward? This isn't a security issue. Your bot isn't saying it is, but it sure reads like one, and someone not paying attention will think this is a security issue.

This makes sense. Thank you for filing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants