Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable Signed URLs when Entry ID taken from URL Param #1509

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

jakejackson1
Copy link
Member

@jakejackson1 jakejackson1 commented Mar 21, 2024

Description

Close this security loophole:

Do not use the Signed PDF URL feature with the Page Confirmation type, as an end user will be able to change the entry ID in the URL and get access to other PDFs. Signed URLs can be safely used with Text or Redirect Confirmation types. Alternatively, non-signed PDF URLs are not vulnerable, and can be safely used in Page Confirmations.

Testing instructions

  1. Setup a form + PDF
  2. Copy the PDF download shortcode and paste it on a WordPress page. Add the signed=1 attribute to the shortcode
  3. Edit the form's default confirmation and set up a Page Confirmation
  4. Choose the WordPress page you added the shortcode to
  5. Add entry={entry_id} to the Query String setting
  6. Submit the test form and verify the PDF Download Link doesn't include the standard signed URL parameters

Checklist:

  • I've tested the code.
  • My code is easy to read, follow, and understand
  • My code follows the accessibility standards.
  • My code has proper inline documentation / docblocks.

Additional Comments

Copy link

codecov bot commented Mar 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.97%. Comparing base (9173274) to head (8ae3007).

Additional details and impacted files
@@             Coverage Diff              @@
##           development    #1509   +/-   ##
============================================
  Coverage        76.97%   76.97%           
============================================
  Files              244      244           
  Lines            12739    12740    +1     
  Branches           370      370           
============================================
+ Hits              9806     9807    +1     
  Misses            2925     2925           
  Partials             8        8           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jakejackson1 jakejackson1 force-pushed the signed-url-security-issue branch from bbc0de3 to 01581be Compare March 21, 2024 05:56
@jakejackson1 jakejackson1 force-pushed the signed-url-security-issue branch from 01581be to 8ae3007 Compare March 21, 2024 06:07
@jakejackson1 jakejackson1 merged commit fd0baa1 into development Mar 21, 2024
16 checks passed
@jakejackson1 jakejackson1 deleted the signed-url-security-issue branch March 21, 2024 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant