Ticket #1660 (new enhancement)

Opened 5 years ago

Last modified 9 months ago

Reliance on $input->submitted in various managers allows invalid submission

Reported by: zbateson Assigned to: demian
Priority: normal Milestone:
Component: SGL - Manager Severity: need feedback
Keywords: validation submitted Cc:

Description

Problem Description

In at least a few managers, user input is validated based on the assumption $req->get('submitted') is not empty, rather than checking what action was passed. This method of validating user input can allow a malicious user to deliberately send a form with a valid action name, but without sending 'submitted' and cause the form to pass as validated. This can also occur without malicious intent (<input type="image" />).

Problem Scenarios

1) With a form using <input type="image" name="submitted" value="submit" />, Firefox sends the value of 'submitted', while IE doesn't. IE users would submit the form without validating it.
2) A malicious user could rename the submitted button, or set its value to nothing before submitting a form (or otherwise send without 'submitted' using any number of techniques, but this one is easy to illustrate).

javascript:void(document.form[0].submitted.value = '');


Proposed Solution

Overview

Validation functions specific to actions, automatically executed by the controller in a similar fashion to actions. Maintain backward-compatibility for easy integration with existing modules.

Implementation

New variable added to SGL_Manager, $_aValidationsMapping, contains an array of action names mapped to any number of validation functions. This allows for more code reuse if for example 'insert' and 'update' required the same validations, _valid_submit may be used for both. If 'update' required specialized validation in addition to _valid_submit, and an additional _valid_update could be created.

$_aValidationsMapping = array('actionName' => array() /* str function names */ );


do_validate function created in SGL_Manager which calls validate first, then checks if $_aValidationsMapping has action-specific validation functions defined for the current action, calling _valid_functionName() on $this.

PATCH

This solution is included in the attached libValidation.patch, and example usage is also included in ContactUsMgr?.php

Attachments

libValidate.patch (2.0 kB) - added by zbateson on 07/28/08 01:01:30.
ContactUsMgr.php (10.9 kB) - added by zbateson on 07/28/08 01:02:04.
moduleContactUs.patch (5.8 kB) - added by zbateson on 07/28/08 01:11:01.

Change History

07/28/08 01:01:30 changed by zbateson

  • attachment libValidate.patch added.

07/28/08 01:02:04 changed by zbateson

  • attachment ContactUsMgr.php added.

07/28/08 01:11:01 changed by zbateson

  • attachment moduleContactUs.patch added.

08/03/08 06:32:11 changed by demian

  • milestone changed from 0.6.6 to 0.9.0 - refactoring.

zbateson - thanks for your input, I think this degree of change should happen in trunk (0.9) rather than bugfix. i need to resolve some plugin work first then will look at validation after.

02/08/10 13:11:20 changed by demian

  • milestone changed from 2.0 to 0.9.1 - polishing.

02/08/10 15:07:02 changed by demian

  • type changed from defect to enhancement.

02/26/13 12:48:49 changed by demian

  • milestone deleted.