Skip to content

Text and Textarea HTML-encode values into the database on save #2044

Description

@agarzon

Version: 4.18.1, also on master.

Text::prepareRequestValue() and Textarea::prepareRequestValue() call htmlspecialchars() on the
save path, so the database stores entities instead of what the user typed. escapeValue() passes
doubleEncode: true, so every save re-encodes the previous encoding.

Evidence

Saving musique d'ambiance & "quotes" twice through the admin form. Values read back from the
database column with Eloquent, not from the rendered page:

value in DB
typed musique d'ambiance & "quotes"
after 1st save musique d'ambiance & "quotes"
after 2nd save musique d'ambiance & "quotes"

It grows by one layer per save, without limit.

src/UI/src/Fields/Text.php:67

protected function prepareRequestValue(mixed $value): mixed
{
    if (\is_string($value)) {
        return $this->isUnescape() ? $value : $this->escapeValue($value);
    }

    return $value;
}

Textarea.php:34 is the same. WithEscapedValue.php:30 defaults $doubleEncode to true.

Email, Url and Phone extend Text and inherit this. For Url it breaks query strings:
?a=1&b=2 is stored as ?a=1&b=2.

Impact

The admin panel hides it — the value is escaped on save, escaped again on render, and the browser
decodes one layer back, so the form looks correct. The corruption is only visible when the column is
read directly: a JSON API, an export, or another application on the same database. Ours serves quiz
content over REST, and clients received musique d'ambiance.

->unescape() is not a workaround

It is one flag for two different concerns. Turning off the save-time escape also turns off the
output escape, and both outputs are unescaped sinks:

  • resolvePreview() feeds {!! $slot !!} in components/table/td.blade.php:2 (index and detail).
  • Textarea::resolveValue() feeds {!! $value !!} in moonshine::fields.textarea, whose
    ComponentSlot is Htmlable, so {{ }} does not escape it either.

So the documented escape hatch swaps data corruption for stored XSS. The docs describe unescape()
only as "disables the escaping of HTML tags in the field value" — they do not say the value is escaped
on save, or that the method also removes the output escape.

This was already accepted as a bug, for one field

#1894 reported that a password containing ' was escaped before hashing, so it hashed incorrectly.
#1895 fixed it with Password::isUnescape() => true. Same root cause, fixed for one field only.

#1989 identified the general problem ("escaping was incorrectly placed in the storage pipeline") but
was closed as too large to review. There is no open issue tracking it, hence this one.

Suggested fix

Two options, smallest first:

  1. Pass doubleEncode: false at the two prepareRequestValue() call sites. One line. Nothing changes
    for clean text, and the corruption stops compounding.
  2. Split unescape() into its save-side and render-side halves — e.g. add unescapeOnSave(),
    defaulting to today's behaviour. Applications that serve their data outside Blade can then stop the
    storage mutation while keeping the XSS protection added in XSS in Markdown and Textarea fields #1148. Additive and backwards compatible.

(2) is the actual fix; (1) bounds the damage in the meantime. Happy to open a PR for either.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions