Re: [pbs-devel] [PATCH proxmox-backup 2/4] ui: show consent banner before login

[Gabriel Goller <g.goller@proxmox.com>]

On 22.05.2024 17:21, Thomas Lamprecht wrote:
>Am 22/05/2024 um 15:19 schrieb Gabriel Goller:
>> diff --git a/www/index.hbs b/www/index.hbs
>> index 824268e3..8a065a94 100644
>> --- a/www/index.hbs
>> +++ b/www/index.hbs
>> @@ -38,6 +38,7 @@
>>  	UserName: "{{ UserName }}",
>>  	defaultLang: "{{ language }}",
>>  	CSRFPreventionToken: "{{ CSRFPreventionToken }}",
>> +	consentText: `{{ consentText }}`,
>
>My knowledge about handlebars template and our integration of them is a bit
>rusty, but are we sure that above does not allow code injection that can
>alter the UI in some odd way, less an issue for users but might allow easily
>to replace our product trademarks and other barriers that ensure that our
>product stay economically viable without having to modify the code?

Yes it does.
For example if I paste this line into consent.txt:

     ${alert(1)}

it gets executed and you get the alert window.

BUT: I just discussed this a bit with Max and we found a
solution:
Using double quotes!

Handlebars per default escapes some characters on the server-side [0],
namely '"', '<' and '>'. This makes it impossible to escape this string
(As you can't get "outside" of the string without using '"'):

     consentText: "{{ consentText }}",

To allow newlines in the double quotes, we just need to escape them
again, as they won't work out the box as with backticks.

With my limited knowledge of XSS Injections, I think this looks quite
good now :)


[0]: https://github.com/sunng87/handlebars-rust/blob/1c92d492a644a563ec3bd4699b6427c86bb4eae9/src/support.rs#L43


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel