Skip to content

fix(core): avoid caching missing locale data#69345

Merged
thePunderWoman merged 1 commit into
angular:mainfrom
SkyZeroZx:fix/locale-data
Jun 15, 2026
Merged

fix(core): avoid caching missing locale data#69345
thePunderWoman merged 1 commit into
angular:mainfrom
SkyZeroZx:fix/locale-data

Conversation

@SkyZeroZx

@SkyZeroZx SkyZeroZx commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Only cache locale data loaded from the global locale registry when an actual locale entry is found.

This prevents attacker-controlled missing locale identifiers from being retained indefinitely when locale lookup falls back to a parent locale or the built-in English locale, avoiding unbounded memory growth in locale-aware pipes and formatters.

I think this can be classified like GHSA-48r7-hpm6-gfxm

@angular-robot angular-robot Bot added the area: core Issues related to the framework runtime label Jun 13, 2026
@ngbot ngbot Bot added this to the Backlog milestone Jun 13, 2026
@SkyZeroZx SkyZeroZx marked this pull request as ready for review June 14, 2026 12:54
@pullapprove pullapprove Bot requested a review from crisbeto June 14, 2026 12:54
@JeanMeche JeanMeche requested review from JeanMeche and alan-agius4 and removed request for crisbeto June 14, 2026 17:30
@alan-agius4 alan-agius4 added the target: patch This PR is targeted for the next patch release label Jun 14, 2026
Comment thread packages/core/src/i18n/locale_data_api.ts
Comment thread packages/core/src/i18n/locale_data_api.ts Outdated
Only cache locale data loaded from the global locale registry when an actual locale entry is found.

This prevents attacker-controlled missing locale identifiers from being retained indefinitely in SSR when locale lookup falls back to a parent locale or the built-in English locale, avoiding unbounded process memory growth in locale-aware pipes and formatters.

@JeanMeche JeanMeche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the additions

@alan-agius4

Copy link
Copy Markdown
Contributor

For visibility whlist this change is good, this IMHO falls under “hardening” and doesn’t warrant to be backported, reason being is that if the API are used as intended and documented, unknown locales in SSR will bootstrap the app with the configured default locale. Unknown locales are never passed to Angular.

@SkyZeroZx

SkyZeroZx commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

For visibility whlist this change is good, this IMHO falls under “hardening” and doesn’t warrant to be backported, reason being is that if the API are used as intended and documented, unknown locales in SSR will bootstrap the app with the configured default locale. Unknown locales are never passed to Angular.

Theoretically, the argument could come from a queryParams or a dynamic property (which I don't think would be unusual, as subpath or query in general is standard), and if it uses locale directly without any sanitization (which as a user I would think should be safe), I would consider it a security problem.
For example a minimal component

@Component({
  imports: [DecimalPipe],
  template: `
    <p>{{ amount | number: '1.2-2' : locale }}</p>
  `,
})
export class Locale  {
  private route = inject(ActivatedRoute);

  amount = 10000;
  locale = this.route.snapshot.queryParamMap.get('locale') ?? 'en-US';
}

On the other hand, I'm not very familiar with internationalization, but I don't think there was any documentation indicating that doing this wasn't recommended (nor would I expect it to cause a problem in SSR if that were the case).

@thePunderWoman thePunderWoman merged commit 98f42ea into angular:main Jun 15, 2026
24 checks passed
@thePunderWoman

Copy link
Copy Markdown
Contributor

This PR was merged into the repository. The changes were merged into the following branches:

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

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants