Repository navigation
Conversation
added 3 commits
October 27, 2026 16:02
Fix A: Bind all untrusted values in DSpaceControlledVocabulary as XPath variables via XPathVariableResolver instead of string interpolation, so entryID, parent and child ids and match/best-match filters can no longer alter the XPath expression. Fix B: Honor metadata.hide.* when deciding which fields to export in DSpaceCSV (gated by bulkedit.ignore-on-export.include-metadata-hide, default true); expose MetadataExposureServiceImpl.CONFIG_PREFIX. Fix C: Neutralize CSV formula injection in DSpaceCSVLine by prefixing a single quote to cell values that begin with = + - @ or a leading tab/CR.
…tests Fix C: make formula neutralization reversible. Export now prefixes a single quote per value (not per cell) when the value starts with = + - @ TAB CR or a single quote; import strips one leading quote per value token. Export followed by re-import is now the identity for every value, including multi-valued cells and values that legitimately start with a trigger character or a quote. Fix B: only fold metadata.hide.* keys that actually indicate hidden (true or submitter), skipping those explicitly set to false; use substring instead of split (the prefix was used as a regex). Tests: add a CSV export/re-import round-trip test, cover per-value and leading quote neutralization, and add a getMatches XPath-injection test for parity.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security — XPath injection + CSV metadata.hide + CSV formula injection
Part of dataquest-dev/dspace-customers#1104 (R36), see #1083. Three related hardening fixes in the controlled-vocabulary lookup and the bulk-edit CSV export.
A. XPath injection in controlled-vocabulary lookups
DSpaceControlledVocabularybuilt XPath expressions by string interpolation, and the anonymous-reachableentryID/parent lookups passed untrusted request parameters straight into them (upstream only hardened the label-search path, notentryID). All lookups —getMatches,getBestMatch,getNode,getChoicesByParent,getParentChoice— now bind untrusted input as XPath variables viasetXPathVariableResolver; no untrusted value is concatenated into an expression. Apostrophe-bearing ids/labels now match correctly (previously mangled by an'replacement).B. CSV export now honors
metadata.hideDSpaceCSVexport ignored themetadata.hide.*configuration, exposing hidden metadata. Export now foldsmetadata.hide.*fields into the ignore set, gated bybulkedit.ignore-on-export.include-metadata-hide(default true) and only when the field is actually hidden (valuetrueorsubmitter;falseis respected), matchingMetadataExposureService. Force export (exportAll) still includes everything. (CONFIG_PREFIXonMetadataExposureServiceImplis promoted topublic static finalto share the constant.)C. CSV formula-injection neutralization — round-trip safe
Spreadsheet applications can execute a cell beginning with
= + - @(etc.) as a formula. Export now prefixes such values (and values already starting with') with a single quote. Crucially, since DSpace CSV is a bulk-edit round-trip format, import removes one leading quote per value, so export→re-import is the identity for every value (including multi-valued cells and values that legitimately start with a quote). This avoids the data-corruption that a naive export-only guard would cause onmetadata-import.Documented behavior note (bulkedit.cfg): because the import-side removal is unconditional, importing a hand-authored or pre-existing CSV whose value legitimately starts with a single quote drops one leading quote — the standard, accepted cost of symmetric CSV-injection neutralization.
Tests
' or '1'='1,x'] | //node | foo[') select nothing; exact and apostrophe matches still work (DSpaceControlledVocabularyTest,farm.xmlfixture).DSpaceCSVTest).DSpaceCSVIT).Note: the dspace-api unit/IT tests could not be executed in the dev environment (a pre-existing stale test-kernel issue affecting all unit tests); they compile + checkstyle clean and are intended to run in CI.
🤖 Generated with Claude Code