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.

Sign up for GitHub

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

Jump to bottom

Fix for Reference Number Client-side Bypass on Payment Processing #7353

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix for Reference Number Client-side Bypass on Payment Processing #7353

wants to merge 2 commits into from

Conversation

ameghana
Copy link

This pull request addresses a critical vulnerability identified in the payment processing module of the OpenEMR application. Specifically, the vulnerability allowed client-side bypassing of input validation for the reference number during payment submissions.

Fixes #7351

Short description of what this resolves:
This pull request resolves a security vulnerability in the OpenEMR application where the reference number for payment entries could be bypassed using client-side manipulation. The absence of strict server-side validation for the reference number allowed entries without valid references, compromising the integrity of payment data and violating ASVS V5.1.4 requirements.

Changes proposed in this pull request:

  • Server-Side Validation: By implementing server-side checks, the application no longer solely depends on client-side validation. This protects against data tampering using client-side tools.
  • Blocking Unvalidated Submissions: The updated PHP script ensures that any payment submission that does not meet the criteria (e.g., missing reference number for checks) is not processed. This is crucial for maintaining data integrity and compliance with business rules.
  • User Feedback: The JavaScript updates improve user interaction by providing immediate feedback when validation fails, guiding users to correct submissions and improving overall user experience.

@ameghana
Update front_payment.php
8454b9e 
To fix Client side bypassing vulnerability for empty check number.
adunsulag
adunsulag requested changes Apr 18, 2024
View reviewed changes
Copy link
Sponsor Member

@adunsulag adunsulag left a comment

Choose a reason for hiding this comment

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

I'll leave the rest of this review to one of the other admin's as I'm not familiar with this code area, but you should put back the copyright notice you removed.

interface/patient_file/front_payment.php Outdated
* @copyright Copyright (c) 2006-2020 Rod Roark <rod@sunsetsystems.com>
* @copyright Copyright (c) 2017-2018 Brady Miller <brady.g.miller@gmail.com>
* @copyright Copyright (c) 2024 Stephen Waite <stephen.waite@open-emr.org>
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Why are you removing someone's copyright...

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the confusion. I have reverted the changes made with copyright statements in my new commit.

@ameghana ameghana requested a review from adunsulag April 20, 2024 03:04
@adunsulag
Copy link
Sponsor Member

Bumping this to the next patch as one of the other admin's will need to review this.

@adunsulag adunsulag added this to the 7.0.2.2 milestone Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adunsulag adunsulag Awaiting requested review from adunsulag

Assignees
No one assigned
Labels
Status: Needs Review
Projects
None yet
Milestone
7.0.2.2
Development

Successfully merging this pull request may close these issues.

Fix for Reference Number Client-side Bypass on Payment Processing
2 participants
@ameghana @adunsulag