Skip to content

Render downloaded changelog as text - #607

Merged
ExtremeFiretop merged 1 commit into
ExtremeFiretop:devfrom
maghuro:security/render-changelog-as-text
Sep 24, 2026
Merged

ExtremeFiretop merged 1 commit into
ExtremeFiretop:devfrom
maghuro:security/render-changelog-as-text

Conversation

@maghuro

@maghuro maghuro commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Render the downloaded firmware changelog as plain text instead of inserting it into the page as HTML.

Problem

FetchChangelog() retrieves the changelog using dataType: 'text', but then inserts the returned data using jQuery .html().

Although the response is fetched as text, .html() causes any HTML markup contained in that text to be interpreted by the browser.

The changelog content originates outside the WebUI and should be displayed as changelog text, not parsed as page markup.

Fix

Replace the HTML string construction with a real pre element whose contents are assigned using .text().

The new statement is:

$('#changelogData').empty().append($('<pre>').text(data));

This preserves the existing preformatted changelog display while ensuring the downloaded content is rendered literally.

Result

HTML-like content in the downloaded changelog is displayed as text rather than interpreted as markup.

Normal changelog formatting and whitespace remain preserved by the pre element.

@ExtremeFiretop

ExtremeFiretop commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

@Martinski4GitHub

Thoughts on this?
Technically it might be real, but we trust the upstream source changelog content don't we? That's why we always rendered the download changelog the way we do.

This would protect us from what exactly? From RMerlin trying to hack us through his changelog?
If RMerlin was trying to hack us, he would likely do it through the firmware, no?

@ExtremeFiretop

Copy link
Copy Markdown
Owner

Just an FYI.

I don't see any practical differences between merging this or keeping it as is myself.
In the security field they would likely say "this lacks exploitability" it's real on paper but lacks any real practical exploitation.

We could merge it just to say we did, but i also think this might be another example of hardening something without much purpose, or changing something just to change.

There's multiple upstream sources in the MerlinAU code that we inherently trust. but those trusted sources are selected by us as trusted. Anything RMerlin publishes from an official source is trusted. Such has his website, his Sourceforge and FW Update VPS.

I probably put this as an exploit at the bottom of the barrel 😂

@ExtremeFiretop ExtremeFiretop left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'll merge this in as a minor hardening/cleanup improvement, but I won't count it / mention it / credit it as a bug fix or security fix in release notes. As it has no demonstrated or realistic exploitability in the current design

@ExtremeFiretop
ExtremeFiretop merged commit 6ee931d into ExtremeFiretop:dev Sep 24, 2026
2 checks passed
@maghuro

maghuro commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

I'll merge this in as a minor hardening/cleanup improvement, but I won't count it / mention it / credit it as a bug fix or security fix in release notes. As it has no demonstrated or realistic exploitability in the current design

I know we trust the source(s), but there's always a chance they could be compromised. Better safe than sorry. 🙂

@ExtremeFiretop

ExtremeFiretop commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

I'll merge this in as a minor hardening/cleanup improvement, but I won't count it / mention it / credit it as a bug fix or security fix in release notes. As it has no demonstrated or realistic exploitability in the current design

I know we trust the source(s), but there's always a chance they could be compromised. Better safe than sorry. 🙂

instantly when I hear that I go... True... On paper.
But no one is hacking his Sourceforge to add some HTML to a changelog.

If they hacked his sourceforge to a point that can replace the changelog, at that point they would also have access to much more valuable targets, such as the firmware repository itself on Sourceforge and the checksums inside.

You would reach a way wider audience with whatever attack your doing by replacing the firmware zips and checksums inside than try to Inject some HTML into the changelogs, and hope some MerlinAU users click download the lastest changelog in the WebUI.

So again, I merged it in as a cleanup, but I never did and still don't consider this much of real exploitable risk

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