Skip to content

Fix - Validate upload field_id to prevent unauthenticated upload restriction bypass - #1671

Open
lihsaa591 wants to merge 6 commits into
developfrom
fix/upload-field-id-validation
Open

lihsaa591 wants to merge 6 commits into
developfrom
fix/upload-field-id-validation

Conversation

@lihsaa591

Copy link
Copy Markdown
Contributor

All Submissions:

Changes proposed in this Pull Request:

Security fix for an unauthenticated upload restriction bypass reported via Global Payments' bug bounty (internal ref: themegrill/everest-forms-pro#1232).

Root cause
wp_ajax_nopriv_everest_forms_upload_file is public by design (front-end forms). ajax_validate_form_field() only verified that the form exists and is published, and never that the submitted field_id belongs to that form. With a non-existent field_id, $this->field_data was empty, so the field's "Allowed File Extensions" list was skipped and get_extensions() fell back to all WordPress-allowed types minus $blacklist. $blacklist had htm but not html, so an .html file was accepted into uploads/everest_forms_uploads/tmp/ and served from the site origin (stored XSS).

Fix

  1. ajax_validate_form_field() now rejects the request unless field_id exists in the form and is of type file-upload or image-upload. This also covers remove_file.
  2. Added html, xhtml, shtml, phtml to $blacklist as defense in depth.

Closes # .

How to test the changes in this Pull Request:

Setup: create a published form with a File Upload field restricted to pdf,jpg (note its field id from the form markup, e.g. abc-1) and its form id.

  1. Valid upload: on the front end, upload a .jpg through the field. Expected: succeeds, and the file appears in wp-content/uploads/everest_forms_uploads/tmp/.
  2. Restricted type on real field (logged out):
    curl -F action=everest_forms_upload_file -F form_id=<ID> -F field_id=<REAL_FIELD_ID> -F file=@test.html http://site/wp-admin/admin-ajax.php
    Expected: File type is not allowed.
  3. Bypass attempt (logged out): same request with field_id=does-not-exist. Expected: Something went wrong, please try again. (success:false), and no file in tmp/. Before the fix this returned a URL-able .html file.
  4. Same as step 3 with a field_id of a non-upload field in the same form (e.g. a text field). Expected: rejected.
  5. Same as step 3 with a File Upload field that has no extension restriction, uploading test.html. Expected: File type is not allowed.
  6. Remove file: upload a file, then delete it using the dropzone remove button. Expected: works. Repeat the everest_forms_remove_file call with a bogus field_id. Expected: rejected.
  7. Image Upload field: upload a .png. Expected: succeeds.
  8. Submit a complete form with an uploaded file. Expected: entry saved, and the file is moved to its permanent folder.

Types of changes:

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (modification of the currently available functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully ran tests with your changes locally?
  • Have you updated the documentation accordingly?

Note: the image-upload field does not enforce the image-only type list server-side when no extensions are set (client-side only). Out of scope here; may be a follow-up.

Changelog entry

Fix - Validate field ID in file upload AJAX handlers to prevent bypass of allowed file type restrictions.

@tg-autopilot
tg-autopilot requested a lite review from Copilot October 9, 2026 07:18
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

QA suite — failed ❌

1 of 2 checks failed. The rest passed.

0 passed · 1 failed · 1 skipped · 0 flaky · 68s


What failed

1. authenticate

What should happen: Logged in as "admin" at http://127.0.0.1:9400 but wp-admin never rendered, with no login error and no PHP fatal on the page.
Landed on: http://127.0.0.1:9400/wp-login.php
Page began: Log In Powered by WordPress Username or Email Address Password Remember Me Lost your password? ← Go to QA Test Site

This was retried 1 time and failed every time.

1 screenshot and 1 trace of this failure are in the report linked at the bottom.

Technical detail

Location: tests/e2e/auth.setup.ts:108

Likely cause: the element never appeared, so the test gave up waiting for it. Check the selector at tests/e2e/auth.setup.ts:108. Either this change altered the markup it looks for, or the page needs a state (a menu, a widget, demo content) that a fresh site never seeds.

  106 |         '(display_errors off). A redirect to somewhere unexpected means a plugin is ' +
  107 |         'intercepting admin init.',
> 108 |     ).toBeVisible({ timeout: 30_000 });
      |       ^
  109 |
  110 |     fs.mkdirSync(path.dirname(STORAGE_STATE), { recursive: true });
  111 |     await context.storageState({ path: STORAGE_STATE });

See it for yourself

Download the full report (qa-suite-everest-forms-1671.zip). Unzip it and open qa-report.html in any browser — it shows each failure with screenshots of the page at the moment it broke, and what the run did and did not check.

Replaying a failure step by step (developers)

The archive also carries a Playwright trace — every click, the page at each step, network and console. From the unzipped folder:

npx playwright show-report playwright-report

It needs that command rather than opening the file directly: a trace viewer cannot start from a file:// page.

Automated check — no AI involved. It runs the tests in this branch.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused changes address the documented bypass, and no unresolved issues were identified.

0 open findings

What changed in this PR

This PR closes the described unauthenticated upload restriction bypass in Everest Forms’ upload handlers.

Changes:

  • Rejects field IDs that do not identify an upload field in the submitted form.
  • Adds HTML-related extensions to the upload blacklist.
File Description
includes/​abstracts/​class-evf-form-fields-upload.php Validates upload field IDs and expands the extension blacklist.

🧠 Review effort: Lite


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@lihsaa591 lihsaa591 self-assigned this Oct 9, 2026
@lihsaa591

Copy link
Copy Markdown
Contributor Author

@saurab018 Please verify the PR

@tg-autopilot

Copy link
Copy Markdown
Contributor

Build for 489eb155 is ready 🛎️

⬇️ Download everest-forms.zip (11M)

Installs directly via Plugins → Add New → Upload Plugin.
Link expires in 30 days · updated Oct 9, 2026 1:08 PM +0545

A later push only rebuilds this if its commit message includes #build-zip.

@lihsaa591

Copy link
Copy Markdown
Contributor Author

CI note: Code sniff (PHP 7.4) fails before it checks any code, and the cause isn't in this PR. It fails on other open PRs too.

  1. The workflow uses the deprecated actions/cache@v2, which GitHub auto-fails.
  2. After that, composer install fails because composer.lock is inconsistent. Some locked packages need PHP 8.1+ and others need PHP 7.x.

I ran phpcs locally on the changed file. There are no new violations: 83 findings before and after, none on the changed lines.

I'm leaving the CI changes out of this security fix. They need a separate PR to bump the cache action and regenerate composer.lock. This PR can be reviewed and merged on its own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants