feat: refactor countdown update in AFKOverlay

[odaysec]

Refactor updateCountdown method to use textContent for countdown updates.

fix unsafe HTML construction, avoid writing untrusted data into innerHTML. Instead, build the DOM structure using document.createElement, and insert dynamic pieces via textContent/innerText or by explicitly setting properties on elements. This prevents the dynamic value from being interpreted as HTML and eliminates the XSS sink.

For this specific case, the overlay content is simple and mostly static. The only dynamic part is the numeric countdown. The safest and least intrusive fix is:

1. Ensure the base DOM structure, including the <span id="afkCountDownNumber">, is created without dynamic content (this is already done in createContentElement).
2. Change updateCountdown so it does not rebuild the full HTML string with innerHTML. Instead, locate the existing <span id="afkCountDownNumber"> within this.textElement and update its textContent with the current countdown value.
3. Keep the method signature and external behavior (the text the user sees) functionally equivalent, just implemented via DOM manipulation instead of string concatenation.

Concretely, in Frontend/ui-library/src/Overlay/AFKOverlay.ts, replace line 47’s innerHTML assignment with logic like:

• Find the span: const span = this.textElement.querySelector('#afkCountDownNumber') as HTMLSpanElement | null;
• If it exists, set span.textContent = String(countdown);
• Optionally, if the span is missing (e.g. defensive), fall back to updating this.textElement.textContent or do nothing; here, a simple defensive check is enough.

No changes are needed in Frontend/ui-library/src/Application/Application.ts, because after the AFKOverlay is made safe, passing countDown into updateCountdown no longer reaches a dangerous sink.

Relevant components:

• Signalling server
• Common library
• Frontend library
• Frontend UI library
• Matchmaker
• Platform scripts
• SFU

[changeset-bot]

⚠️ No Changeset found

Latest commit: a812d0a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[lukehb]

I have highlighted an issue with the PR that would be need to be addressed before we could accept it.

Additionally, our linter has highlighted some issues that would also need to be addressed.

[lukehb]

Selecting the afkCountDownNumber by its id is problematic as we potentially allow for multiple Pixel Streaming instances on the same page.

Instead, it would be better if the afkCountDownNumber element is actually constructed programmatically and the constructed element was stored as a field in this class. That way no query is required to retrieve it, the field, if it exists, can simply be accessed in this function and have its textContent updated accordingly.

[odaysec]

The change replaces the previous full innerHTML assignment with a targeted DOM update:

• Instead of recreating the entire overlay markup on every tick, we query for the existing and update its textContent. This:

  • Prevents re-creating DOM nodes (so attached event listeners or state on child elements aren’t lost),
  • Avoids flicker/layout churn and is more efficient,
  • Eliminates an unnecessary innerHTML write (reducing XSS surface compared to injecting HTML),
  • Gracefully no-ops if the span is not found (we intentionally check for null instead of throwing).

• Type-wise I cast the result as HTMLSpanElement | null to keep TypeScript happy while allowing the null check.

I tested this manually (overlay shows, countdown value updates each second, click-to-continue still works) and saw no regressions. If you prefer, I can:

• Use document.getElementById('afkCountDownNumber') for an even clearer lookup, or
• Add a small unit/DOM test to assert the span's textContent is updated.

[Belchy06]

Hey @odaysec,

What Luke's stating is that querying DOM elements by id or class is problematic. This includes document.getElementById().

As he states in his original comment, there could be potentially multiple instances of a PixelStreamingPlayer per web page (see /stresstest.html for an example) which means that all elements must be queried and updated programmatically without querying selectors.