Skip to content

harden: add output encoding in typography.js (CWE-79) - #860

Closed
anupamme wants to merge 1 commit into
inc2734:mainfrom
anupamme:fix-repo-unitone-typography-base-font-size-xss
Closed

anupamme wants to merge 1 commit into
inc2734:mainfrom
anupamme:fix-repo-unitone-typography-base-font-size-xss

Conversation

@anupamme

Copy link
Copy Markdown

The typography panel accepts user input for base font size settings through the baseFontSizeInput state variable without strict input validation. This input is stored in the WordPress database via the saveSettings function and later rendered in the admin interface. While the value is primarily used in CSS contexts, the presence of dangerouslySetInnerHTML usage in the same file for notices indicates potential XSS vectors if user-controlled data reaches these render paths without proper sanitization. This is defence-in-depth at App/Controller/Manager/src/js/panels/typography.js:166 rather than a vulnerability I can show is exploitable here — it makes the failure mode explicit and bounded. Close it freely if the pattern is intentional.

Reference: CWE-79

What changed

  • App/Controller/Manager/src/js/panels/typography.js

Verification

No automated check could be run against this repository, so this change is unverified beyond review. Please treat it as a suggestion.


Automated security fix by OrbisAI Security

Automated security fix generated by OrbisAI Security
@inc2734

inc2734 commented Sep 24, 2026

Copy link
Copy Markdown
Owner

Thank you for the suggestion. I am closing this PR because it breaks supported base font size values. The default is the string 16px (Settings.php), and the Typography input also stores strings such as 20px. The new number-only check turns these values into undefined, so reopening the panel leaves the Base Font Size field empty and the preview no longer reflects the saved value.

The proposed XSS path is not demonstrated: the base font size is used through getRootFontSize and React style properties, while the dangerouslySetInnerHTML notice uses a translation and fixed <code> tags. The base font size does not reach that HTML sink. This change also leaves the saved value and frontend CSS output unchanged, so it does not address those paths. If there is a reproducible injection path, please share the input, sink, and reproduction steps so we can address it at the appropriate boundary.

@inc2734 inc2734 closed this Sep 24, 2026
@anupamme

Copy link
Copy Markdown
Author

Thanks for tracing the existing flow. I agree that the current patch is incorrect because 16px/20px are legitimate values, and the number-only validation breaks the settings UI.

I withdraw that change. I’ll separately verify whether an attacker-controlled base-font-size value can actually reach an HTML sink. If I can establish a reproducible source → sink path, I’ll open a focused PR with the exact payload and reproduction steps. Otherwise, I’ll consider the original finding a false positive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants