No escaping with html()->a() and other methods

[decadence]

Today I discovered that elements you create with html() helper are not properly escaped.

For example documentation says that second argument is text:

function a($href = null, $text = null)

But if you look at source code you see that method calls html method on A:

public function a($href = null, $contents = null)
{
        return A::create()
            ->attributeIf($href, 'href', $href)
            ->html($contents);
}

Therefore user may thinks (as I thought) that text he passes will be escaped but it will not and this can lead to bad things.
Is this by design? My thought that everything should be escaped by default. And if you want html you can call html method by hand.
No warning in documentation is found.

[raveren]

First of all, it floors me that issues are disabled on this repository.

I feel it's at the very least disinformative to the users.

[raveren]

And yeah:

{{ html()->label('<script>alert(123)</script>') }}

This is class is writing unescaped strings to the page, it's super scary 👻

A real-life security concern, and there's no mention of this in the documentation, so I can only assume this is accidental.

Or am I supposed to escape every. string. myself? It's non-realistic to expect to do that without fault from human developers.

It's why Blade escapes everything by default.

Finally, if user really needs verbatim text, there's already a way with Laravel: Htmlable instance which wraps text and tells the ubiquitous e() to not escape it:

{{ html()->label(
		new Htmlable('<script>alert("I really wish to inject js here!")</script>')
) }}

P.S. thanks God, at least the values of inputs are escaped

[francoism90]

@raveren @decadence Disclaimer: I'm not a Spatie employee nor did I create this package. But I think it's working as intended.

Take this example (welcome.blade.php):

{{ html()->div()->class('p-9')->open() }}

    {{ html()->div()->text("<script>alert('foo')</script>") }} // output: <script>alert('foo')</script> (plain text)
    {{ html()->div("<script>alert('foo')</script>") }} // output: Alert dialog

{{ html()->div()->close() }}

The reason for this behavior, is that the last one offers features like this:

{{ html()->label('Username <strong>*</strong>') }} // I actually recommend doing this with `->child()` instead

// {{ html()->element('script')-> .. }} // I do not recommend this, but in theory one could write scripts with the html helper.

See https://github.com/spatie/laravel-html/blob/95c58b71f69cf15fb5991321a6eeb65d3f9737ce/src/BaseElement.php#L298C6-L301C6 and

laravel-html/src/BaseElement.php

Line 402 in 95c58b7

public function render()

for the implementation. Basically text($text) works like {{ $foo }}.

It shouldn't lead to any issues when the code doesn't contain any user input:

{{ html()->div()->class('p-9')->open() }}
    {{ html()->label('About')->for('form.about')->class('label') }}
    {{ html()->textarea()->wireModel('form.about')->class('textarea') }}

    WARNING! Unsafe example when outputting `form.about`:

    {{ html()->div($form->about)->class('p-3 w-full h-12') }}

    Safer example:

    {{ html()->div()->text($form->about)->class('p-3 w-full h-12') }}

    Safer example with markdown/HTML (using something like stevebauman/purify or Laravel's markdown parser helper):

    {{ html()->div($form->about_purified)->class('p-3 w-full h-12') }}
{{ html()->div()->close() }}

@freekmurze @sebastiandedeyne I'll try to make a PR, because I've seen multiple things missing or not working in the docs (if you're agreeing with the given examples). But maybe you already have plans for it? :)

[raveren]

Thank you for this well thought out input!

I still firmly believe the best approach (or rule of thumb) is that everything is escaped by default - and only data that's explicitly set to be used "as is" should be so.

This is an example of how it could be done with existing tools if the html() Spatie functionality would escape by default.

{{ html()->div(
	new Illuminate\Support\HtmlString("<script>alert('foo')</script>")
) }

[francoism90]

@raveren You could overrule the render() with a mixin or macro. Add something like the purify example, and you can make sure only safe elements are passable. :)