Skip to content

Commit d8e84cf

Browse files
committed
fix(framework): improve themeRoot validation (#13354)
Fix themeRoot validation to require explicit origin allowlist via meta tag and separate configuration storage from validated URL usage. Problem: - themeRoot URLs were not properly validated for security - getThemeRoot() mixed raw configuration with validated URLs - Missing validation for relative paths and URL formats Solution: 1. Require meta tag for all themeRoot usage: <meta name="sap-allowed-theme-origins" content="https://cdn.example.com"> - Comma-separated list of allowed origins - Wildcard "*" to allow any origin - Legacy "sap-allowedThemeOrigins" (camelCase) supported - Same-origin URLs allowed when meta tag present 2. Separate configuration from validation: - getThemeRoot() returns raw configured value (unchanged) - validateThemeRoot() performs security checks and normalization - DOM link creation uses validated URL: {validatedRoot}/UI5/Base/baseLib/{theme}/css_variables.css 3. Enhanced validation: - Absolute URLs: check origin against allowlist - Relative paths (./path, ../path): resolve to current origin - Absolute paths (/path): resolve to current origin - Add trailing slash if missing - Append /UI5/ to create proper theme asset path - Return undefined for invalid/unauthorized URLs 4. Proper error handling: - Log warning when validation fails - No DOM link created for invalid themeRoot - Graceful fallback to default theme behavior
1 parent d5a574c commit d8e84cf

3 files changed

Lines changed: 52 additions & 24 deletions

File tree

packages/base/src/InitialConfiguration.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import merge from "./thirdparty/merge.js";
22
import { getFeature } from "./FeaturesRegistry.js";
33
import { DEFAULT_THEME } from "./generated/AssetParameters.js";
4-
import validateThemeRoot from "./validateThemeRoot.js";
54
import type OpenUI5Support from "./features/OpenUI5Support.js";
65
import type { FormatSettings } from "./config/FormatSettings.js";
76
import AnimationMode from "./types/AnimationMode.js";
@@ -156,7 +155,7 @@ const parseURLParameters = () => {
156155
const normalizeThemeRootParamValue = (value: string) => {
157156
const themeRoot = value.split("@")[1];
158157

159-
return validateThemeRoot(themeRoot);
158+
return themeRoot;
160159
};
161160

162161
const normalizeThemeParamValue = (param: string, value: string) => {

packages/base/src/config/ThemeRoot.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,11 @@ const setThemeRoot = (themeRoot: string): Promise<void> | undefined => {
4343

4444
currThemeRoot = themeRoot;
4545

46-
if (!validateThemeRoot(themeRoot)) {
47-
console.warn(`The ${themeRoot} is not valid. Check the allowed origins as suggested in the "setThemeRoot" description.`); // eslint-disable-line
48-
return;
49-
}
50-
5146
return attachCustomThemeStylesToHead(getTheme());
5247
};
5348

54-
const formatThemeLink = (theme: string) => {
55-
return `${getThemeRoot()!}Base/baseLib/${theme}/css_variables.css`; // theme root is always set at this point.
49+
const formatThemeLink = (theme: string, validatedThemeRoot: string) => {
50+
return `${validatedThemeRoot}Base/baseLib/${theme}/css_variables.css`;
5651
};
5752

5853
const attachCustomThemeStylesToHead = async (theme: string): Promise<void> => {
@@ -62,7 +57,20 @@ const attachCustomThemeStylesToHead = async (theme: string): Promise<void> => {
6257
document.head.removeChild(link);
6358
}
6459

65-
await createLinkInHead(formatThemeLink(theme), { "sap-ui-webcomponents-theme": theme });
60+
const themeRoot = getThemeRoot();
61+
62+
if (!themeRoot) {
63+
return;
64+
}
65+
66+
const validatedThemeRoot = validateThemeRoot(themeRoot);
67+
68+
if (!validatedThemeRoot) {
69+
console.warn(`The ${themeRoot} is not valid. Check the allowed origins as suggested in the "setThemeRoot" description.`); // eslint-disable-line
70+
return;
71+
}
72+
73+
await createLinkInHead(formatThemeLink(theme, validatedThemeRoot), { "sap-ui-webcomponents-theme": theme });
6674
};
6775

6876
export {

packages/base/src/validateThemeRoot.ts

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,65 @@
1+
const isSSR = typeof document === "undefined";
2+
3+
const getLocationHref = () => {
4+
if (isSSR) {
5+
return "";
6+
}
7+
8+
return window.location.href;
9+
};
10+
111
const getMetaTagValue = (metaTagName: string) => {
212
const metaTag = document.querySelector(`META[name="${metaTagName}"]`),
313
metaTagContent = metaTag && metaTag.getAttribute("content");
414

515
return metaTagContent;
616
};
717

8-
const validateThemeOrigin = (origin: string) => {
9-
const allowedOrigins = getMetaTagValue("sap-allowedThemeOrigins");
18+
const validateThemeOrigin = (origin: string, isSameOrigin: boolean = false) => {
19+
const allowedOrigins = getMetaTagValue("sap-allowed-theme-origins") ?? getMetaTagValue("sap-allowedThemeOrigins"); // Prioritize the new meta tag name
1020

11-
return allowedOrigins && allowedOrigins.split(",").some(allowedOrigin => {
12-
return allowedOrigin === "*" || origin === allowedOrigin.trim();
13-
});
14-
};
21+
// If no allowed origins are specified, block.
22+
if (!allowedOrigins) {
23+
return false;
24+
}
1525

16-
const buildCorrectUrl = (oldUrl: string, newOrigin: string) => {
17-
const oldUrlPath = new URL(oldUrl).pathname;
26+
// If it's same-origin (relative URL resolved to current page), allow it when there's any meta tag present
27+
// The presence of the meta tag indicates the user wants to use theme roots
28+
if (isSameOrigin) {
29+
return true;
30+
}
1831

19-
return new URL(oldUrlPath, newOrigin).toString();
32+
return allowedOrigins.split(",").some(allowedOrigin => {
33+
return allowedOrigin === "*" || origin === allowedOrigin.trim();
34+
});
2035
};
2136

2237
const validateThemeRoot = (themeRoot: string) => {
2338
let resultUrl;
39+
let isSameOrigin = false;
2440

2541
try {
2642
if (themeRoot.startsWith(".") || themeRoot.startsWith("/")) {
2743
// Handle relative url
2844
// new URL("/newExmPath", "http://example.com/exmPath") => http://example.com/newExmPath
2945
// new URL("./newExmPath", "http://example.com/exmPath") => http://example.com/exmPath/newExmPath
3046
// new URL("../newExmPath", "http://example.com/exmPath") => http://example.com/newExmPath
31-
resultUrl = new URL(themeRoot, window.location.href).toString();
47+
resultUrl = new URL(themeRoot, getLocationHref()).toString();
48+
isSameOrigin = true;
3249
} else {
3350
const themeRootURL = new URL(themeRoot);
3451
const origin = themeRootURL.origin;
52+
const currentOrigin = new URL(getLocationHref()).origin;
53+
54+
// Check if the absolute URL is same-origin
55+
isSameOrigin = origin === currentOrigin;
3556

36-
if (origin && validateThemeOrigin(origin)) {
57+
if (origin && validateThemeOrigin(origin, isSameOrigin)) {
3758
// If origin is allowed, use it
3859
resultUrl = themeRootURL.toString();
3960
} else {
40-
// If origin is not allow and the URL is not relative, we have to replace the origin
41-
// with current location
42-
resultUrl = buildCorrectUrl(themeRootURL.toString(), window.location.href);
61+
// If origin is not allowed, return undefined to indicate validation failed
62+
return undefined;
4363
}
4464
}
4565

@@ -50,6 +70,7 @@ const validateThemeRoot = (themeRoot: string) => {
5070
return `${resultUrl}UI5/`;
5171
} catch (e) {
5272
// Catch if URL is not correct
73+
return undefined;
5374
}
5475
};
5576

0 commit comments

Comments
 (0)