Do you need to sanitize radio buttons or checkboxes when using POST?

[SamC]

Hi everyone, just writing a new tutorial as a 'break' from learning PHP. It involves making a contact form. In brief, the part of the form with the radios and a checkbox:

contact.php

    <fieldset class="form-group">
      <div class="row">
        <legend class="col-form-label col-sm-2 pt-0">Choose a colour</legend>
        <div class="col-sm-10">
          <div class="form-check">
            <input class="form-check-input" type="radio" name="gridRadios" id="gridRadios1" value="red" checked>
            <label class="form-check-label" for="gridRadios1">Red</label>
          </div>
          <div class="form-check">
            <input class="form-check-input" type="radio" name="gridRadios" id="gridRadios2" value="blue">
            <label class="form-check-label" for="gridRadios2">Blue</label>
          </div>
          <div class="form-check">
            <input class="form-check-input" type="radio" name="gridRadios" id="gridRadios3" value="green">
            <label class="form-check-label" for="gridRadios3">Green</label>
          </div>
        </div>
      </div>
    </fieldset>

    <div class="form-group row">
      <div class="col-sm-2"><a href="#">Terms & Conditions</a> (required)</div>
      <div class="col-sm-10">
        <div class="form-check">
          <input class="form-check-input" type="checkbox" name="checkboxAgree" id="checkboxAgree">
          <label class="form-check-label" for="checkboxAgree">Tick to agree</label>
        </div>
      </div>
    </div>

...and the template where the form posts to (very early days work in progress):

contact-process.php

<?php namespace ProcessWire;

// Sanitize user inputs
$name = $sanitizer->text($input->post->name);
$email = $sanitizer->email($input->post->email);
$message = $sanitizer->textarea($input->post->message);

// Radio button values
$allowed_values = ['red', 'green', 'blue'];
/**
 * Only allow allowed values above, returns NULL if anything other
 * than the values in the above array
 */
$radio_value = $sanitizer->option($input->post->gridRadios, $allowed_values);

// DOES THIS NEED TO BE SANITIZED??
$checkbox_agree = $input->post->checkboxAgree;

$submitted = $input->post->submitted;

// Concantenate all the values
$str = 'Name: ' . $name;
$str .= '<br>';
$str .= 'Email: ' . $email;
$str .= '<br>';
$str .= 'Message: ' . $message;
$str .= '<br>';
$str .= 'Radio value: ' . $radio_value;
$str .= '<br>';
$str .= 'Checkbox agree: ' . $checkbox_agree;
$str .= '<br>';
$str .= 'Form is submitted: ' . $submitted;
echo $str;

Form outputs:

Name: Sam
Email: sam@test.com
Message: Hi, this is a test message...
Radio value: blue
Checkbox agree: on
Form is submitted: 1

So would it be necessary to sanitize the radio buttons so only red, blue or green are submitted, and also, what about the checkbox?

Could a user manipulate the POST array to send values other than the ones I want?

Maybe just being paranoid but don't want to give out information that I haven't checked first.

Thanks  

[kongondo]

36 minutes ago, SamC said:

So would it be necessary to sanitize the radio buttons so only red, blue or green are submitted

It's always good to sanitise and validate. You don't lose anything. The only things you don't sanitise are passwords (just validate) and usually, a submit button (just check if post sent).

Regarding the radio buttons, you are using $sanitizer->option which is validating the options sent. Implicitly, you are sanitising here as well since you provide the array of sanitised values.

38 minutes ago, SamC said:

and also, what about the checkbox?

Yes. Sanitise it too as necessary. 

But, these depend on what you are going to do with the values. General practice though is to sanitise at the earliest opportunity.

There is also encoding of html entities if you are going to be echoing back input values. So, this...

echo $sanitizer->entities($str);

 

[kongondo]

50 minutes ago, SamC said:

Could a user manipulate the POST array to send values other than the ones I want?

Oh yes. Very much so. Try it yourself. Edit a value in chrome dev and inspect what's been sent.

I forgot to mention. If you are not doing it already, use CSRF protection and a honey pot.

 

Edited April 11, 2018 by kongondo

[neosin]

@SamC   Best practice is to sanitize everything a user sends to server, never trust users. It is easy to hack forms using just your browser "developer tools" it lets you change values of everything including all form values. Using your example colors values, the values could be changed to do multiple things from "using unsuported other values" to injecting SQL or PHP code for whatever nefarious purpose.

The only time you can relax is if you have predefined values and match those values to what is sent to the server.

For example in your case, on form submit you could compare the gridRadios array to a PHP array of values to see if there is any match, if so then data is valid and proceed otherwise reject submission as invalid.

but 99.9% of the time it's best to just sanizite user input

 

[bernhard]

Hi @SamC, great to see you are making progress

2 things:

1) I'm always sending forms to the same page an in that template I do something like this:

if($config->ajax AND $input->post->submit) {
  // process form
}

// regular template code

Usually this makes the setup easier.

2) As you see there is a LOT to consider with forms. That's why I built the RockForms module using Nette Forms, because that takes care of all this stuff. I'm not saying "stop learning and use my module" here  But the nette docs might be an interesting read for you. You'll come across the things mentioned above (CSRF, validation etc.). And besides that, it is the only library I found that does validation on the client side and on the server side with one single setup of the form  

[SamC]

Thanks everyone for the input here, it's very useful  

1 hour ago, bernhard said:

1) I'm always sending forms to the same page an in that template I do something like this:

if($config->ajax AND $input->post->submit) {
  // process form
}

// regular template code

Usually this makes the setup easier.

Regarding this, how does this stop duplicate submissions? I mean, the code doesn't run when the form hasn't been submitted ($input->post_submit), but when the form has been submitted, what happens on refresh? The last action was POST so doesn't the form attempt to POST again?

This is one reason why I started using the PRG pattern, although tbh, I'd rather post to the same page, makes things simpler, especially if you're keeping form fields populated i.e. the ones that did validate.

Regarding the nette docs, I didn't really understand them! I'm a mere beginner @bernhard  

[bernhard]

10 minutes ago, SamC said:

Regarding this, how does this stop duplicate submissions? I mean, the code doesn't run when the form hasn't been submitted ($input->post_submit), but when the form has been submitted, what happens on refresh? The last action was POST so doesn't the form attempt to POST again?

yes, you can $session->redirect() after processing your form, then it will clear your post data.

next thing to take care of is when the form is submitted and there is a validation error on the server, the form for the user needs to be re-populated with the values already submitted

[SamC]

Ah I see. I was thinking of:

1) Submitting the form
2) Post to form-process.php
3) Sanitize values, see which ones validate (with valitron)
4) Stick the valid ones in session as an array of form data along with which ones have an error, like:

[
'name' => 'Sam',
'email' => 'fail',
'message' => 'hi this is a message',
'phone_number' => 'fail'
]

5) Redirect back to original form
6) Repopulate the valid fields from session, error class on the rest

If all fields are valid, after (3), just email form, redirect back to contact page (with now blank contact form) with success message.

Is this a goofy approach? I don't see any other way if POST data has been cleared, either by session->redirect() (post to same page) or by a PRG pattern.