Test strings with preserveHTML = false:

$makeTestStringFx = static fn ($v) => $v . ' <b>"\' &lt;';
$htmlValues = [
    $makeTestStringFx('d') => $makeTestStringFx('dTitle'),
    $makeTestStringFx('u') => $makeTestStringFx('uTitle'),
];

before:

after:

For unknown reasons to me, multiple values are escaped by default by get values API in https://github.com/fomantic/Fomantic-UI/blob/2.9.4/src/definitions/modules/dropdown.js#L2012. This is addressed in de27144 to fix #3199 (comment).

I tested both (t/f) preserveHTML and this PR fixes all #3199 issues.

Thank you for the extensive review.

I extracted changes I consider safe into #3212 to minimize the LoC changed of this PR to discuss it later.

Here is a jsfiddle where XSS takes place inside FUI when preserveHTML is false and the dropdown is NOT multiple: https://jsfiddle.net/8rabcw71/1/

Paste "<script>alert('gotcha!')</script>" into the second dropdown and it gets executed immedialty. This way an attacker could add someting less obvious like <img onload="some_evil_stuff()" style="visibility:hidden;"><b>hi there</b> Also the dropdown kinda gets stuck afterwards....

Good catch, fixed.

Two things left for me atm:

1st

The fix from fddfe7a should also be applied to

so it also becomes

html = settings.templates.addition(module.add.variables(message.addResult, settings.templates.escape(value, settings)));

2nd

As i explained the meaning of preserveHTML in #3205 (comment)
The check at

It should be either

module.add.userSuggestion(!settings.preserveHTML
	? module.escape.htmlEntities(query)
	: query);

or

module.add.userSuggestion(settings.preserveHTML
	? query
	: module.escape.htmlEntities(query));

But the related change was implemented in #3224, so it might be adjusted there, if you want the other PR to be merged first

1st

Fixed by me, my mistake. I now checked all .html( inputs.

2nd

You are worried about a995f2f. No, it makes sense - with preserveHTML = true we want to preserve HTML, but not from user input :), thus we encode the input first and then preserve it everywhere else as other values. With preserveHTML = false the input is text already, no need to encode it, we encode it only for HTML render.

No, it makes sense - with preserveHTML = true we want to preserve HTML, but not from user input :),

Sure thing 👍🏼 That's why it was basically always encoded before just in case.

thus we encode the input first and then preserve it everywhere else as other values. With preserveHTML = false the input is text already, no need to encode it, we encode it only for HTML render.

Alright. I debugged it down now more deeply and i can confirm, after your latest fixes, that the usersuggestion is now safely encoded when finally being added to the DOM 🥳

Just to clarify/summarize (for myself, if i ever need to rethink/reconfirm this in some later situation/refactoring)

1. The query variable, when preserveHTML is false, is still unsanitized when provided to the module.filter() function because it just gets the .val() from the input, no matter what there might be HTML inside
2. It is even still unsanitized, when preserveHTML is false, when provided to the module.add.userSuggestion function

Those 2 situations, in addition to the found XSS use case yesterday, made me not believing the code was correct... until today 😉

3. BUT: inside module.add.userSuggestion the value GETS encoded when preserveHTML is false: The two LOC where settings.templates.addition() is called (that was fixed in the latest commits)

@mvorisek Thanks for being patient with me (again) and explaining all my confusion

I will now look into the other modules usages for the escape method which were also adjusted in this PR to finally merge your both PRs