Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:95680
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain ohgaki.net designates 180.42.98.130 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <5b72e9da-068a-bc79-82c2-f36f723f42bb@gmail.com>
References: <CAGa2bXarw2jTj0yhXpywWNQOJLR2pdhfaqSdwj_btT2giEL15Q@mail.gmail.com>
 <CAGa2bXaA+uvh_wpRAHeiv+Xb-JOrTAkyoe3jzq3FVCEbfja_5w@mail.gmail.com>
 <232F1604-2211-4351-B830-EDC958A25D6D@strojny.net> <CAGa2bXb2y4afY28rwdcjgxUEkr41ZPMMdj4dUtMDVtJLBONAbA@mail.gmail.com>
 <2de35db0-9974-cc96-83dd-3d2dbd48f7f8@lsces.co.uk> <CAGa2bXYG1Or9+SGf3Rm4Yi5KUQ1mwnffTSSzuwARpy4bFfXB=w@mail.gmail.com>
 <5b72e9da-068a-bc79-82c2-f36f723f42bb@gmail.com>
Date: Tue, 6 Sep 2016 10:56:17 +0900
Message-ID: <CAGa2bXa+9raNXmg90rsfi+yt_9HMBtWXWORy3f_014Teg+CcBg@mail.gmail.com>
To: Rowan Collins <rowan.collins@gmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [RFC][VOTE] Add validation functions to filter module
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

Hi Rowan,

On Fri, Sep 2, 2016 at 7:37 PM, Rowan Collins <rowan.collins@gmail.com> wrote:
> On 02/09/2016 11:11, Yasuo Ohgaki wrote:
>>
>> Taking care of tampered data by business logic will reduce both
>> readability and maintainability. And more importantly, make code
>> less secure because programmers tend to focus on logic
>> in model, not input data validations.
>
>
> This certainly makes sense. I guess the challenge is that in order to know
> if data has been tampered, you need to have some knowledge of the expected
> format. That expectation depends on what data you're expecting, which
> depends - ultimately - on the domain objects being modelled.
>
> More specifically, though, it depends on the interaction design - in an HTML
> context, the forms being presented. So the validation needs knowledge of the
> form controls - e.g. if a select box was shown, and the value is not from
> the known list of options, the input has been tampered with.
>
> If that's the case, the logical place to build the validation is into a form
> builder. At which point you've probably got a complex architecture in
> userland, and filter_* functions are unlikely to be a natural fit.
>
> If somebody's *not* using a library to build the form (e.g. they're laying
> out the HTML by hand), are they likely to set up the complex validation
> settings needed by the filter_* functions?

I agree HTML form is the most important input. However, apps
has to deal with other inputs like GET/COOKIE/JSON/HTTP Request
headers/Inputs from other subsystems. Form builder does not work
well for input data other than forms...

Even with great form builder, letting validation to input code has
advantages.

One issue is "data handling is better to break down into 2 parts",
logic and format. Since model's responsibility is to handle logic,
programmers try to handle logic in model. It's not bad at all with
this. However, there are many apps have poor format checks
because of this.

2nd issue is coverage. "model deals with data that is handled by
the model." Data validation covered by a model does not match with
application inputs. There are many vulnerabilities "Oops, this value
is not validated" even for data like ID. This is not limited to PHP apps.
e.g. https://groups.google.com/forum/#!topic/rubyonrails-security/ly-IH-fxr_Q

3rd issue is location. Input data validation is better to be done as
soon as possible. When application accepts input, programmers
know what the possible inputs, and could cover all inputs. i.e.
Controller is the best place for input format validation.
"Data" domain can be defined as a single definition, but making
sure it has "Valid Format" by models/libraries is harder than it seems.
If it's easy, we don't have this many vulnerabilities in PHP apps.

The best practice of input validation is
"Establish and maintain control over all of your inputs."
http://cwe.mitre.org/top25/#Mitigations

There are many possible implementations for this.
If we apply DbC to application, we have to validate external inputs
somewhere to keep contract and make code works w/o any
problems. Natural location for input validation is controller.
Internal redirects without validation could be serious bug
such as authentication bypass. Input validation can mitigate
such bug also if validation is done at controller.


BTW, I don't think everyone has to validate input very strict
manner. It is ok to validate like

<?php
// Define loose input validation

$server_def = array(
   // REQUEST HEADER
   'HTTP_ACCEPT' => $text64b_spec, // Text up to 64 bytes
   'HTTP_ACCEPT_ENCODING' => $text64b_spec,
   'HTTP_ACCEPT_LANGUAGE' => $text64b_spec,
   'HTTP_USER_AGENT' => $text512b_spec, // Text up to 512 bytes
   'HTTP_COOKIE' => $text1k_spec, // Text up to 1024 bytes
   'REQUEST_URI' => $text4k_spec, // Text up to 4096 bytes
);

$cookie_def = array(
    // COOKIE
   'PHPSESSID' => $phpsessid_spec,
  'uid' => $uid_spec,
  'last_access' => $int_spec,
 );

$get_def = array(
    // GET
   'id'  => $id_spec,
   'other_id' => $id_spec,
   'type1' => $alnum32b_spec, // Alpha numeric up to 32 bytes
   'type2'=> $alnum32b_spec,
);

$post_def = array(
    // POST
   'text1'    => $input_tag_spec, // <input type=text> default up to
                                              // 512 bytes text
   'text2'    => $input_tag_spec,
   'select1'    => $select_tag_spec, // <select> values default up to
                                                   // 64 bytes text
   'select2'    => $select_tag_spec,
   'radio1'      => $radio_tag_spec, // <radio> values default up to
                                                  // 64 bytes text
   'radio2'      => $radio_tag_spec,
   'submit'    => $submit_tag_spec,
   'textarea1k'   => $textarea1k_spec, // <textarea> values default up
                                                       // to 1K text,
allow newline
   'textarea100k' => $textare100k_spec, // <textarea> values default
                                                          // up to
100K text, allow newline
   'CSRF_TOKEN' => $alnum32b_spec, // Alpha numeric up to 32 bytes
   // and so on
);

$_SERVER = filter_require_var_array($_SERVER);
$_COOKIE = filter_require_var_array($_COOKIE);
$_GET = filter_require_var_array($_GET);
$_POST =filter_require_var_array($_POST);
?>

Since new string validator validates encoding and control chars
including newline, this validation rejects tampered inputs include
broken encoding, null char injection, newline injection and other
CNTRL char injection.

This definition is loose and weak, but we are sure "broken encoding,
null char injection, newline injection and other CNTRL char injection"
_FREE_ at least, except newline injections by textarea inputs.  It's
very important we are sure free from certain vulnerabilities even if
output code/library/subsystem is supposed to sanitize data for 100%
safety.

We still have these kind of vulnerabilities.
e.g.  I've fixed mail header injections via extra headers parameter recently.
https://bugs.php.net/bug.php?id=68776
We cannot be sure if 3rd party modules are free from encoding, CNTRL
injection attacks.

How far developers would like to validate by the input validation code
is up to developers. The more validate strictly, more secure and
increases chance to avoid hidden vulnerabilities in your code or other
people's code. i.e. Framework, library, or even PHP/subsystems like
database.

Regards,

P.S.
To people against this RFC,

How many of you are against the idea of this RFC?
(I don't think Rowan against basic idea, BTW)

--
Yasuo Ohgaki
yohgaki@ohgaki.net