fix: resolve XML-Enc elements by DOM traversal instead of XPath - #141
Open
elbuo8 wants to merge 1 commit into
Open
fix: resolve XML-Enc elements by DOM traversal instead of XPath#141elbuo8 wants to merge 1 commit into
elbuo8 wants to merge 1 commit into
Conversation
decryptKeyInfo located elements with XPath expressions that were built by
string concatenation and scoped by a leading "//". Both properties made the
lookups behave differently from how they read.
"//" is descendant-or-self over the whole document regardless of the context
node passed to select(), so lookups that appear scoped were document-wide:
- the wrapped key was taken from the first CipherValue anywhere in the
document rather than from the EncryptedKey being processed
- DigestMethod was resolved by an absolute path, so with several
EncryptedKey elements it could return a digest declared by one key while
the ciphertext came from another
- the absolute DigestMethod path also matched nothing for documents using
EncryptedData/KeyInfo/RetrievalMethod, where EncryptedKey sits outside
KeyInfo, silently defaulting oaepHash to sha1
Predicates were also assembled by concatenating the RetrievalMethod URI into
the expression text, so an Id containing a quote changed what the expression
meant instead of being compared as a value.
lib/dom-select.js walks the DOM explicitly. Document values are compared,
never concatenated into anything that gets parsed, and scope is a node
enforced by the walk itself. EncryptionMethod and CipherValue are now read
from one resolved EncryptedKey, so they always describe the same key.
Two input-validation changes come with this:
- a RetrievalMethod URI must be a same-document fragment reference.
substring(1) removed the first character whatever it was, so "/ek1" and
"Xek1" both resolved to Id "ek1".
- a duplicated Id is reported rather than resolved to its first match.
Duplicate Ids are invalid XML and resolving them makes the result depend
on document order.
xpath moves to devDependencies; the runtime no longer parses expressions.
Verified with a packed tarball installed via --omit=dev, where
require('xpath') throws MODULE_NOT_FOUND and both fixtures still decrypt.
65 passing (44 before). Each property is pinned by a test that also asserts
the previous XPath behaviour for contrast, and 11 mutations of the traversal
and the guards were each confirmed to fail a named test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
decryptKeyInfo located elements with XPath expressions that were built by string concatenation and scoped by a leading "//". Both properties made the lookups behave differently from how they read.
"//" is descendant-or-self over the whole document regardless of the context node passed to select(), so lookups that appear scoped were document-wide:
Predicates were also assembled by concatenating the RetrievalMethod URI into the expression text, so an Id containing a quote changed what the expression meant instead of being compared as a value.
lib/dom-select.js walks the DOM explicitly. Document values are compared, never concatenated into anything that gets parsed, and scope is a node enforced by the walk itself. EncryptionMethod and CipherValue are now read from one resolved EncryptedKey, so they always describe the same key.
Two input-validation changes come with this:
xpath moves to devDependencies; the runtime no longer parses expressions. Verified with a packed tarball installed via --omit=dev, where require('xpath') throws MODULE_NOT_FOUND and both fixtures still decrypt.
65 passing (44 before). Each property is pinned by a test that also asserts the previous XPath behaviour for contrast, and 11 mutations of the traversal and the guards were each confirmed to fail a named test.
By submitting a PR to this repository, you agree to the terms within the Auth0 Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.
Description
References
Testing
Checklist