Make all persisted form builds immutable

Created on 9 June 2015, over 9 years ago
Updated 28 August 2024, 2 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 16 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.

Production build 0.71.5 2024