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

HTML search: sanitise 'searchindex.js' contents #13099

Closed
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
37e92ab
HTML search: sanitise `searchindex.js` contents
jayaddison Nov 3, 2024
58ca700
Add CHANGES.rst entry
jayaddison Nov 3, 2024
1554551
Fixup: code-indentation correction
jayaddison Nov 4, 2024
056c85b
Perf: refactor `sanitiseRecursive`
jayaddison Nov 4, 2024
6a38587
Perf: additional `sanitiseRecursive` refactoring
jayaddison Nov 4, 2024
015ecb9
Prototype (unbenchmarked): remove nested-arrays from `searchindex.js`
jayaddison Nov 4, 2024
a3a1a31
Fixup: add missing counter-increment
jayaddison Nov 5, 2024
db3aaf5
Tests: regenerate HTML search test fixtures
jayaddison Nov 5, 2024
a72b1ff
Fixup: ignore entry-number keys when re-opening search index from file
jayaddison Nov 5, 2024
13581d1
Tests: update expectations
jayaddison Nov 5, 2024
94857f7
Typing: add parameterized typehints to `Counter` variables
jayaddison Nov 5, 2024
ca7490d
Linting: resolve `ruff` PERF102 suggestions
jayaddison Nov 5, 2024
6df348f
Revert "Linting: resolve `ruff` PERF102 suggestions"
jayaddison Nov 5, 2024
dab072f
Revert "Typing: add parameterized typehints to `Counter` variables"
jayaddison Nov 5, 2024
dbafb5f
Revert "Tests: update expectations"
jayaddison Nov 5, 2024
5ab5f1d
Revert "Fixup: ignore entry-number keys when re-opening search index …
jayaddison Nov 5, 2024
7a75647
Revert "Tests: regenerate HTML search test fixtures"
jayaddison Nov 5, 2024
8c2f7e2
Revert "Fixup: add missing counter-increment"
jayaddison Nov 5, 2024
26ec589
Revert "Prototype (unbenchmarked): remove nested-arrays from `searchi…
jayaddison Nov 5, 2024
c0af102
Tests: add test coverage to demonstrate mutating a nested array element
jayaddison Nov 5, 2024
0248ccc
sanitiseRecursive: implement recursive array sanitisation
jayaddison Nov 5, 2024
6542c8b
Refactor: extract common `evaluate` function path
jayaddison Nov 5, 2024
2ada40e
Refactor: avoid possibiliy of repeat `evaluate` construction
jayaddison Nov 5, 2024
e6ecded
Refactor: `sanitiseRecursive` brevity/performance
jayaddison Nov 5, 2024
6b4b675
Refactor: translate from `sanitiseRecursive` to an iterative approach
jayaddison Nov 5, 2024
ab841c6
Performance: avoid `Object.values` call for `Array` instances
jayaddison Nov 6, 2024
0e61a3c
Performance: omit non-`Object` items from the stack
jayaddison Nov 6, 2024
b2de18a
Refactor: rectify variable name
jayaddison Nov 6, 2024
6aa4f7f
Refactor / performance: consolidate non-emptiness check with stack po…
jayaddison Nov 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ Deprecated
Features added
--------------

* #13098: HTML Search: the contents of the search index are sanitised
(reassigned to frozen null-prototype JavaScript objects), to reduce
the risk of unintended access or modification.
Patch by James Addison

Bugs fixed
----------

Expand Down
18 changes: 15 additions & 3 deletions sphinx/themes/basic/static/searchtools.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,18 @@ const Search = {
(document.body.appendChild(document.createElement("script")).src = url),

setIndex: (index) => {
const stack = [index];
while (stack.length) {
const value = stack.pop();
if (!(value instanceof Object)) continue;
jayaddison marked this conversation as resolved.
Show resolved Hide resolved
if (Array.isArray(value)) {
stack.push(...value);
} else {
stack.push(...Object.values(value));
Object.setPrototypeOf(value, null);
}
Object.freeze(value);
}
Search._index = index;
if (Search._queued_query !== null) {
const query = Search._queued_query;
Expand Down Expand Up @@ -474,7 +486,7 @@ const Search = {
const descr = objName + _(", in ") + title;

// add custom score for some objects according to scorer
if (Scorer.objPrio.hasOwnProperty(match[2]))
if (match[2] in Scorer.objPrio)
score += Scorer.objPrio[match[2]];
else score += Scorer.objPrioDefault;

Expand Down Expand Up @@ -520,13 +532,13 @@ const Search = {
// add support for partial matches
if (word.length > 2) {
const escapedWord = _escapeRegExp(word);
if (!terms.hasOwnProperty(word)) {
if (!(word in terms)) {
Object.keys(terms).forEach((term) => {
if (term.match(escapedWord))
arr.push({ files: terms[term], score: Scorer.partialTerm });
});
}
if (!titleTerms.hasOwnProperty(word)) {
if (!(word in titleTerms)) {
Object.keys(titleTerms).forEach((term) => {
if (term.match(escapedWord))
arr.push({ files: titleTerms[term], score: Scorer.partialTitle });
Expand Down
33 changes: 33 additions & 0 deletions tests/js/searchtools.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,39 @@ describe('Basic html theme search', function() {
expect(nextExpected).toEqual(undefined);
}

describe('index immutability', function() {

it('should not be possible to modify index contents', function() {
eval(loadFixture("cpp/searchindex.js"));

// record some initial state
const initialTitle = Search._index.titles[0];
const initialTitlesProto = Search._index.titles.__proto__;
const initialTerms = Search._index.terms;
const initialDocNames = [...Search._index.docnames];
const initialObject = [...Search._index.objects[''][0]];

// attempt to mutate the index state
try { Search._index.objects[''][0].pop(); } catch {};
try { Search._index.objects[''][0].push('extra'); } catch {};
try { Search._index.docnames.pop(); } catch {};
try { Search._index.docnames.push('extra'); } catch {};
Search._index.titles[0] += 'modified';
Search._index.titles.__proto__ = 'anotherProto';
Search._index.terms = {'fake': [1, 2, 3]};
Search._index.__proto__ = 'otherProto';

// ensure that none of the modifications were applied
expect(Search._index.__proto__).toBe(undefined);
expect(Search._index.terms).toEqual(initialTerms);
expect(Search._index.titles.__proto__).toEqual(initialTitlesProto);
expect(Search._index.titles[0]).toEqual(initialTitle);
expect(Search._index.docnames).toEqual(initialDocNames);
expect(Search._index.objects[''][0]).toEqual(initialObject);
});

});

describe('terms search', function() {

it('should find "C++" when in index', function() {
Expand Down
Loading