Contrib.social

Make all persisted form builds immutable

Open on Drupal.org →
Created on 9 June 2015, almost 10 years ago
Updated 28 August 2024, 9 months ago

Problem/Motivation

  1. To fix a security issue with minimal disruption, SA-CORE-2014-002 → introduced a $form_state['build_info']['immutable'] defined as:
     *     - immutable: If this flag is set to TRUE, a new form build id is
 *       generated when the form is loaded from the cache. If it is subsequently
 *       saved to the cache again, it will have another cache id and therefore
 *       the original form and form-state will remain unaltered.

    #2421503: SA-CORE-2014-002 forward port only checks internal cache → is an open critical issue for making sure the flag is set in all cases that it needs to be.

  2. This raises an interesting question though: when is it ever desirable for the behavior to be anything else? From a logic standpoint, if a request ends up modifying $form or $form_state, isn't that a new form build? And if so, shouldn't that always correspond to a new form build ID?
  3. Continuing this line of thinking, in the language of Domain Driven Design, shouldn't we be thinking of the form build ($form + $form_state) as a Value Object rather than an Entity? In which case, we can express that by changing $form_build_id from a randomly generated string to a hash of $form and $form_state. This would simultaneously enforce immutability while also conserve on redundant KV entries (e.g., repeated submissions with validation errors or of node previews could reuse the same build ID/hash and therefore the same KV entry, so long as the built $form and $form_state are unchanged).

Proposed resolution

Change $form_build_id from a randomly generated string to the hash of the $form and $form_state being persisted.

Remaining tasks

Discuss, patch, review, commit.

User interface changes

None.

API changes

  • $form_state['build_info']['immutable'] removed, since immutability would be baked in.
  • $form_build_id generated later in the build process (just prior to persisting), so hook_form_alter() and friends wouldn't have access to it. This isn't much of a loss, as there shouldn't be any logic based on the build ID anyway. It was always intended solely as an ID for implementing caching/persistence.
📌 Task
Status

Postponed: needs info

Version

11.0 🔥

Component
Form  →

Last updated about 7 hours ago

Created by

🇺🇸United States effulgentsia

Live updates comments and jobs are added and updated live.
  • Security improvements

    It makes Drupal less vulnerable to abuse or misuse. Note, this is the preferred tag, though the Security tag has a large body of issues tagged to it. Do NOT publicly disclose security vulnerabilities; contact the security team instead. Anyone (whether security team or not) can apply this tag to security improvements that do not directly present a vulnerability e.g. hardening an API to add filtering to reduce a common mistake in contributed modules.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024