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

Omise Setting Page, sanitizing input fields before save #104

Merged
merged 2 commits into from
Mar 26, 2019

Conversation

guzzilar
Copy link
Contributor

@guzzilar guzzilar commented Mar 25, 2019

1. Objective

To prevent any suspicious sources from trying to alter Omise options, this pull request is aiming to add a validation and sanitizing the input fields.

Related information:
Internal ticket: T10248
Related issue(s): -

2. Description of change

  • Validating WP Nonce before allowing a particular post-request pass through the rest of the code.
  • Sanitizing inputs before save a new value to the database.

3. Quality assurance

🔧 Environments:

  • PHP version: 7.3.3.
  • WordPress: v5.1.1.
  • WooCommerce: v3.5.7.

✏️ Details:

At the Omise Setting page, you may try to edit a nonce value directly via browser's inspect element feature then, submit the form.

Screen Shot 2562-03-25 at 23 13 49

Omise will raise an error message warning that you cannot update the setting.
Screen Shot 2562-03-25 at 23 06 21

You may also try to update any of the setting's field with the following value:

<script type="text/javascript">alert("Hello! I am an alert box!!");</script>

or

%3Cscript+type=%22text/javascript%22%3Ealert(%22Hello!+I+am+an+alert+box!!%22);%3C/script%3E

The plugin will then sanitize it to

script+type=text/javascriptalert(Hello!+I+am+an+alert+box!!);/script

4. Impact of the change

No

5. Priority of change

High.

6. Additional Notes

Alternatively, I think there is a better way to handle & validate HTTP Request than adding a condition at class-omise-page-settings.php.

Maybe can have a class

$request = new Omise_Http_Request;

print_r( $request->is_post() ); // true

$request->input( 'omise_setting_page_nonce' )->sanitize();
$this->settings->update_settings( $request->to_array() );

Just idea.

@guzzilar guzzilar merged commit 982251d into master Mar 26, 2019
@guzzilar guzzilar deleted the sanitize-form-submission branch March 26, 2019 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants