From f7ca427fad4431bcf252e91bae3e59205992816f Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 16 Sep 2026 22:44:52 +0100 Subject: [PATCH 1/2] FIX: Store snippet results only under names the snippet writes A snippet that builds its result name as it runs, such as the location snippet's "51D_Pos_" + key + "=", left an empty value in session storage under the text of the join, fod_data_51D_Pos_" + key + ", and every request from the tab then sent 51D_Pos_%22+%2B+key+%2B+%22 to the server. The rewrite of the store itself was already right, because the join is copied inside the session storage key, but the empty result written before the snippet runs took the name group literally. A fixed name joined from two strings, as in "51D_Bandwidth" + "=", was stored the same wrong way. The empty result now removes joins between quoted strings, and stores nothing for a name that is joined to a variable, because that name is only known as the snippet runs. A value an earlier script stored under a name holding a quote is no longer sent. The pattern now reads a quoted name as a fixed start joined only to quoted text and plain variable names, and leaves a space before the equals sign out of the name, so any other form is left to write its cookie rather than being stored under a name the pattern did not understand. The template literal form used by the high entropy values snippet was already read correctly and is now covered by tests, as are the forms that are left alone. --- JavaScriptResource.mustache | 37 ++++- README.md | 13 +- tests/template-tests.js | 280 ++++++++++++++++++++++++++++++++++++ 3 files changed, 325 insertions(+), 5 deletions(-) diff --git a/JavaScriptResource.mustache b/JavaScriptResource.mustache index 565739a..f5db93b 100644 --- a/JavaScriptResource.mustache +++ b/JavaScriptResource.mustache @@ -279,6 +279,13 @@ fiftyoneDegreesManager = function() { if(startsWith(nextKey, session51DataPrefix)){ let name = nextKey.substring(session51DataPrefix.length); + // A name holding a quote was stored by an earlier + // version of this script, which took the text of + // a join for a fixed name. No snippet writes a + // result by that name, so it is not sent. + if(name.indexOf('"') !== -1){ + continue; + } if(!Object.prototype.hasOwnProperty.call( fodValues, name)){ fodValues[name] = window.sessionStorage[nextKey]; @@ -796,7 +803,20 @@ fiftyoneDegreesManager = function() { // then process them and perform any call-backs required. if (jsProperties !== undefined && jsProperties.length > 0) { - let valueSetPrefix = new RegExp('document\\.cookie\\s*=\\s*(("([A-Za-z0-9_"\\s\\+]+)\\s*=\\s*"\\s*\\+\\s*([^\\s};]+))|(`([A-Za-z0-9_]+)\\s*=\\s*\\$\\{([^}]+)\\}`))', 'g'); + // The stores a snippet makes, found in its text. Group 3 is the + // name of a quoted store and group 6 the name of a template + // literal store, and groups 4 and 7 start their values. A quoted + // name is a fixed start, optionally joined with + to further + // quoted text and to plain variable names, as in + // "51D_Pos_" + key + "=" and "51D_Bandwidth" + "=". Group 3 then + // holds the text between the outer quotes, so the rewrite below + // puts the same join inside the session storage key and the + // snippet builds the same name there as it would for a cookie. + // Any other form, such as a name joined to a member or a call, + // or a template literal with anything but a fixed name and one + // value, is not matched, and its store is left as it is rather + // than rewritten under a name the pattern did not understand. + let valueSetPrefix = new RegExp('document\\.cookie\\s*=\\s*(("([A-Za-z0-9_]+(?:"\\s*\\+\\s*(?:[A-Za-z_$][A-Za-z0-9_$]*\\s*\\+\\s*)*"[A-Za-z0-9_]*)*)\\s*=\\s*"\\s*\\+\\s*([^\\s};]+))|(`([A-Za-z0-9_]+)\\s*=\\s*\\$\\{([^}]+)\\}`))', 'g'); let session51DataPrefix = sessionKey + "_data_"; {{^_enableCookies}} let sessionSetPatch = 'window.sessionStorage["' + session51DataPrefix + '$3$6"]=$4$7'; @@ -811,12 +831,23 @@ fiftyoneDegreesManager = function() { // result goes to session storage whatever the cookie setting, so // the script gains no cookie write of its own, and // getFodSavedValues falls back to that result for a name the - // cookies do not carry. + // cookies do not carry. A quoted name joined only to further + // quoted text is one fixed name once the joins are removed. A + // quote left after that means a variable is joined in, so the + // name is only known as the snippet runs and nothing is stored + // for it here, because storing the text of the join would send + // the server a name no snippet writes. let storeEmptyValues = function(snippet) { let found; valueSetPrefix.lastIndex = 0; while ((found = valueSetPrefix.exec(snippet)) !== null) { - let valueName = found[3] || found[6]; + let valueName = found[6]; + if (found[3]) { + valueName = found[3].replace(/"\s*\+\s*"/g, ''); + if (valueName.indexOf('"') !== -1) { + continue; + } + } if (!valueName) { continue; } diff --git a/README.md b/README.md index 743efe6..374cc50 100644 --- a/README.md +++ b/README.md @@ -18,13 +18,18 @@ The processJsProperties function in javascript template has a section that uses - **Cookie Assignment**: The expression should start with `document.cookie = ` - **Spaces**: Spaces around the first `=` sign are optional - **Cookie Name**: The name of the cookie should only contain alphanumeric characters, underscores, and must not have spaces +- **Joined Cookie Name**: In double quotes, the name can be a fixed start joined with `+` to further quoted text and to plain variable names, for example `"51D_Pos_" + key + "="` or `"51D_Bandwidth" + "="`. The session storage key is then joined the same way, so it holds the name the snippet builds as it runs - **Assignment with Double Quotes**: The cookie value assignment can use double quotes, and the value should be set programmatically by concatenating a string with a variable or expression -- **Assignment with Backticks**: The cookie value assignment can use backticks for template literals, and the value can be set programmatically using expressions inside `${}` +- **Assignment with Backticks**: The cookie value assignment can use backticks for template literals, and the value can be set programmatically using expressions inside `${}`. The name in a template literal must be fixed - **No Direct Value Assignment**: Directly setting a value within the string is not allowed; values must be set programmatically +A statement that does not follow these rules is not changed, so it still writes its cookie and nothing is kept in session storage for it. + +Before a snippet runs, the script stores an empty value in session storage for each fixed name the snippet writes, which is how the server hears that the snippet ran and stored nothing. A name joined to a variable is only known as the snippet runs, so no empty value is stored for it. + #### Regular Expression: ```javascript -/document\.cookie\s*=\s*(("([A-Za-z0-9_"\s\+]+)\s*=\s*"\s*\+\s*([^\s};]+))|(`([A-Za-z0-9_]+)\s*=\s*\$\{([^}]+)\}`))/g +/document\.cookie\s*=\s*(("([A-Za-z0-9_]+(?:"\s*\+\s*(?:[A-Za-z_$][A-Za-z0-9_$]*\s*\+\s*)*"[A-Za-z0-9_]*)*)\s*=\s*"\s*\+\s*([^\s};]+))|(`([A-Za-z0-9_]+)\s*=\s*\$\{([^}]+)\}`))/g ``` #### Valid Examples: @@ -36,6 +41,8 @@ document.cookie="51D_PropertyName="+screen.height; // No spaces, variable assign document.cookie=`51D_PropertyName=${btoa(JSON.stringify(value))}` // Using a template literal with an expression document.cookie="51D_PropertyName="+profileIds.join("|") // Assigning a value using a joined string of variables document.cookie = `51D_PropertyName=${"True"}`; // Using backticks for programmatic value assignment +document.cookie = "51D_Pos_" + key + "=" + pos.coords[key]; // A name built from a variable as the snippet runs +document.cookie = "51D_Bandwidth" + "=" + value; // A fixed name joined from two strings ``` #### Invalid Examples: @@ -45,6 +52,8 @@ document.cookie = "51D_PropertyName=" + profileIds.join(" ") // Spaces within th document.cookie = " 51D_PropertyName = " + "True"; // Spaces inside the cookie name are not allowed document.cookie = ` 51D_PropertyName =${"True"}`; // Spaces inside the template literal are not allowed document.cookie = `51D_PropertyName=START${window.middle}END`; // Concatenating strings directly within template literals is not allowed +document.cookie = `51D_${key}=${value}`; // A name built inside a template literal is not allowed +document.cookie = "51D_" + item.key + "=" + value; // A name joined to anything but a plain variable name is not allowed ``` --- diff --git a/tests/template-tests.js b/tests/template-tests.js index 22bedde..ccdb736 100644 --- a/tests/template-tests.js +++ b/tests/template-tests.js @@ -1528,6 +1528,286 @@ section('A page whose publisher turned cookies on'); withoutCookies === 1, 'count ' + withoutCookies); } + +// --------------------------------------------------------------------------- +section('Result names the snippets build or join'); +// --------------------------------------------------------------------------- +{ + // The location snippet as the cloud serves it. The name of each + // coordinate is joined to a fixed start as the snippet runs, and the + // error path writes a fixed name. + const LOCATION_SNIPPET = [ + 'if (navigator.geolocation) {', + ' navigator.geolocation.getCurrentPosition(function(pos) {', + ' for (var key in pos.coords) {', + ' document.cookie = "51D_Pos_" + key + "=" + pos.coords[key];', + ' }', + ' // 51D replace this comment with callback function.', + ' }, function(e) {', + ' document.cookie ="51D_Pos_Error=" + encodeURIComponent(e.message);', + ' // 51D replace this comment with callback function.', + ' });', + '}' + ].join('\n'); + + // The high entropy values snippet as the device data carries it, which + // writes its result with a template literal. + const HIGH_ENTROPY_SNIPPET = [ + 'if(navigator.userAgentData){navigator.userAgentData' + + '.getHighEntropyValues(["model","platform","platformVersion",' + + '"fullVersionList"]).then(t=>{document.cookie=' + + '`51D_GetHighEntropyValues=${btoa(JSON.stringify(t))}`', + '// 51D replace this comment with callback function.', + '})} else { // 51D replace this comment with callback function.', + '}' + ].join('\n'); + + // The bandwidth snippet's store, which joins two quoted strings to make + // a fixed name. + const JOINED_SNIPPET = + 'var value = "fast"; ' + + 'document.cookie = "51D_Bandwidth" + "=" + encodeURIComponent(value);'; + + const payloadFor = function (body) { + return JSON.stringify({ + location: { javascript: body }, + javascriptProperties: ['location.javascript'] + }); + }; + + // A stored name that no store could give, being one carrying a quote, a + // plus or a template placeholder, is a store under the wrong key. + const wrongKeys = function (tab) { + return Object.keys(tab.session.data).filter(function (k) { + return /["+`]|\$\{/.test(k); + }); + }; + + const geolocation = function (outcome) { + return { + getCurrentPosition: function (success, failure) { + setTimeout(function () { + if (outcome === 'allow') { + success({ coords: { latitude: 51, longitude: -1 } }); + } else { + failure({ message: 'User denied Geolocation' }); + } + }, 0); + } + }; + }; + + // Cookies off, the visitor allows the position. + { + const tab = makeTab(); + const view = pageView(tab, { + model: { _jsonObject: payloadFor(LOCATION_SNIPPET) }, + globals: { navigator: { geolocation: geolocation('allow') } }, + responses: [secondPayload] + }); + await settle(12); + const body = view.endpoint.bodies[0] || ''; + check('a built name is stored under the name the snippet built', + tab.session.data['fod_data_51D_Pos_latitude'] === '51' && + tab.session.data['fod_data_51D_Pos_longitude'] === '-1', + JSON.stringify(tab.session.data)); + check('a built name leaves nothing stored under a wrong key', + wrongKeys(tab).length === 0, + JSON.stringify(wrongKeys(tab))); + check('the fixed name beside a built one still gets its empty result', + tab.session.data['fod_data_51D_Pos_Error'] === '', + JSON.stringify(tab.session.data)); + check('the request carries the built names and their values', + /(^|&)51D_Pos_latitude=51(&|$)/.test(body) && + /(^|&)51D_Pos_longitude=-1(&|$)/.test(body), body); + check('the request carries no name with a quote in it', + body.indexOf('%22') === -1 && body.indexOf('"') === -1, body); + } + + // Cookies off, the visitor refuses. + { + const tab = makeTab(); + const view = pageView(tab, { + model: { _jsonObject: payloadFor(LOCATION_SNIPPET) }, + globals: { navigator: { geolocation: geolocation('deny') } }, + responses: [secondPayload] + }); + await settle(12); + const body = view.endpoint.bodies[0] || ''; + check('the error path stores its fixed name', + tab.session.data['fod_data_51D_Pos_Error'] === + 'User%20denied%20Geolocation', + JSON.stringify(tab.session.data)); + check('the error path leaves nothing stored under a wrong key', + wrongKeys(tab).length === 0, JSON.stringify(wrongKeys(tab))); + check('the error path request carries no name with a quote in it', + body.indexOf('%22') === -1 && + body.indexOf('51D_Pos_Error=') !== -1, body); + } + + // Cookies on, the visitor allows the position. + { + const tab = makeTab(); + const view = pageView(tab, { + model: { + _enableCookies: true, + _jsonObject: payloadFor(LOCATION_SNIPPET) + }, + globals: { navigator: { geolocation: geolocation('allow') } }, + responses: [secondPayload] + }); + await settle(12); + const body = view.endpoint.bodies[0] || ''; + check('a cookie page writes the built names as cookies', + tab.cookies['51D_Pos_latitude'] === '51' && + tab.cookies['51D_Pos_longitude'] === '-1', + JSON.stringify(tab.cookies)); + check('a cookie page stores nothing under a wrong key', + wrongKeys(tab).length === 0, JSON.stringify(wrongKeys(tab))); + check('a cookie page sends no name with a quote in it', + body.indexOf('%22') === -1 && + /(^|&)51D_Pos_latitude=51(&|$)/.test(body), body); + } + + // A template literal name, cookies off. + { + const tab = makeTab(); + const hints = { model: 'Pixel 9', platform: 'Android' }; + const view = pageView(tab, { + model: { _jsonObject: payloadFor(HIGH_ENTROPY_SNIPPET) }, + globals: { + btoa: btoa, + navigator: { + userAgentData: { + getHighEntropyValues: function () { + return Promise.resolve(hints); + } + } + } + }, + responses: [secondPayload] + }); + await settle(12); + const expected = btoa(JSON.stringify(hints)); + const body = view.endpoint.bodies[0] || ''; + check('a template literal name is stored under that name', + tab.session.data['fod_data_51D_GetHighEntropyValues'] === + expected, + JSON.stringify(tab.session.data)); + check('a template literal name leaves nothing under a wrong key', + wrongKeys(tab).length === 0, JSON.stringify(wrongKeys(tab))); + check('the request carries the template literal result', + body.indexOf('51D_GetHighEntropyValues=' + + encodeURIComponent(expected)) !== -1, body); + } + + // A template literal name where the interface is missing, so the + // snippet stores nothing and the empty result is what is sent. + { + const tab = makeTab(); + const view = pageView(tab, { + model: { _jsonObject: payloadFor(HIGH_ENTROPY_SNIPPET) }, + globals: { navigator: {} }, + responses: [secondPayload] + }); + await settle(12); + check('a template literal name gets its empty result', + /(^|&)51D_GetHighEntropyValues=(&|$)/ + .test(view.endpoint.bodies[0] || ''), + view.endpoint.bodies[0]); + } + + // Two quoted strings joined into one fixed name, cookies off. + { + const tab = makeTab(); + const view = pageView(tab, { + model: { _jsonObject: payloadFor(JOINED_SNIPPET) }, + responses: [secondPayload] + }); + await settle(12); + check('a name joined from two strings is stored under the whole name', + tab.session.data['fod_data_51D_Bandwidth'] === 'fast', + JSON.stringify(tab.session.data)); + check('a name joined from two strings leaves no wrong key', + wrongKeys(tab).length === 0, JSON.stringify(wrongKeys(tab))); + check('a name joined from two strings is sent under the whole name', + /(^|&)51D_Bandwidth=fast(&|$)/ + .test(view.endpoint.bodies[0] || ''), + view.endpoint.bodies[0]); + } + + // A space before the equals sign is not part of the name, as a browser + // trims it from a cookie name. + { + const tab = makeTab(); + const view = pageView(tab, { + model: { + _jsonObject: payloadFor( + 'document.cookie = "51D_Spaced =" + "wide";') + }, + responses: [secondPayload] + }); + await settle(12); + check('a space before the equals sign is left out of the name', + tab.session.data['fod_data_51D_Spaced'] === 'wide' && + Object.keys(tab.session.data).filter(function (k) { + return / /.test(k); + }).length === 0, + JSON.stringify(Object.keys(tab.session.data))); + check('a space before the equals sign is left out of the sent name', + /(^|&)51D_Spaced=wide(&|$)/ + .test(view.endpoint.bodies[0] || ''), + view.endpoint.bodies[0]); + } + + // Forms the rewrite does not read are left to write their cookie, and + // nothing is stored for them, rather than a store under a wrong key. + const notRead = [ + ['a name built from a member', + 'var o = { k: "Member" }; ' + + 'document.cookie = "51D_" + o.k + "=" + "1";', + '51D_Member'], + ['a template literal with a built name', + 'var k = "Built"; document.cookie = `51D_${k}=${"1"}`;', + '51D_Built'], + ['a template literal with text after the value', + 'document.cookie = `51D_Tail=${"1"}; path=/`;', + '51D_Tail'] + ]; + for (const [label, snippet, cookieName] of notRead) { + const tab = makeTab(); + const jar = {}; + const view = pageView(tab, { + model: { _jsonObject: payloadFor(snippet) }, + globals: { document: makeCookieDocument(jar) }, + responses: [secondPayload] + }); + await settle(12); + check(label + ' is left to write its cookie', + jar[cookieName] === '1', JSON.stringify(jar)); + check(label + ' stores nothing in session storage', + Object.keys(tab.session.data).filter(function (k) { + return k.indexOf('fod_data_') === 0; + }).length === 0, + JSON.stringify(Object.keys(tab.session.data))); + check(label + ' still lets the round finish', + view.endpoint.count() === 1, 'count ' + view.endpoint.count()); + } + + // A tab that an earlier script left holding a value under a wrong key + // does not send it again. + { + const tab = makeTab(); + tab.session.data['fod_data_51D_Pos_" + key + "'] = ''; + const view = pageView(tab, { responses: [secondPayload] }); + await settle(12); + const body = view.endpoint.bodies[0] || ''; + check('a value stored under a wrong key earlier is not sent', + body.indexOf('%22') === -1 && body.indexOf('51D_Pos_') === -1 && + body.indexOf('51D_testvalue=purple') !== -1, body); + } +} + console.log('\n' + checks + ' checks, ' + failures + ' failures'); process.exit(failures === 0 ? 0 : 1); })(); From 6d92f23696a910b4421387299887246db62132d4 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 17 Sep 2026 06:46:39 +0100 Subject: [PATCH 2/2] TEST: Check the README prints the pattern the template uses The description of this change said a check confirmed the README pattern and the template's were the same, and no such check existed. The template holds the pattern as a single quoted string with its backslashes doubled, so the check halves them and looks for the result in the README. Changing one character of the README pattern turns the check red, which is how it was proved. --- tests/template-tests.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/template-tests.js b/tests/template-tests.js index ccdb736..62d7678 100644 --- a/tests/template-tests.js +++ b/tests/template-tests.js @@ -294,6 +294,25 @@ section('Formatting constraints the other ports\' tests enforce'); try { new vm.Script(s); } catch (err) { ok = false; why = err.message; } check('the rendered script parses, ' + name, ok, why); } + + // The README carries a copy of the pattern processJsProperties uses to + // find the stores a snippet makes, and so does the server side of the + // cloud service. A change to the pattern that leaves the README behind + // sends the next reader, and the next port, to a pattern the template no + // longer uses. The template holds it as a single quoted JavaScript string + // whose backslashes are doubled, so they are halved here to get the text + // the README prints. + { + const held = template.match( + /let valueSetPrefix = new RegExp\('(.*)', 'g'\);/); + const patternText = held && held[1].replace(/\\\\/g, '\\'); + const readme = fs.readFileSync( + path.join(__dirname, '..', 'README.md'), 'utf8'); + check('the README prints the pattern the template uses', + !!patternText && + readme.indexOf('/' + patternText + '/g') !== -1, + JSON.stringify(patternText)); + } } // ---------------------------------------------------------------------------