-1

On my website users can submit a form that contains their name, email, and an amount.

<input type='text'  minlength='2' maxlength='30' spellcheck='false' placeholder='Elon'           autocomplete='off' form='form' required>
<input type='email' minlength='6' maxlength='40' spellcheck='false' placeholder='musk@tesla.com' autocomplete='off' form='form' required>
<input type='number' step='0.01' min='2000' max='99999999.99'       placeholder='$2,000.00'      autocomplete='off' form='form' required>

However, instead of posting the HTML form, the values are parsed in one JS function, which then sends a string containing all of the parameters to another function that creates an AJAX request.

form.onsubmit = function(e){
  const
    children = this.children,
    summary = this.parentNode.parentNode.children[0].innerText.split('.'),
    negotiate = this.parentNode.children[1]
  insert_data(`table=offers
    &name=${children[0].value.toLowerCase()}
    &email=${children[1].value.toLowerCase()}
    &amount=${children[2].value * 100}
    &sld=${summary[0]}
    &tld=${summary[1]}`
  )
  return false
}

function insert_data(parameters, async){
  async = async === undefined || async
  let xhr = window.XMLHttpRequest
    ? new XMLHttpRequest()
    : new ActiveXObject('Microsoft.XMLHTTP')
  xhr.open('POST', 'ajax.php', async)
  xhr.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded;charset=UTF-8')
  xhr.send(parameters)
}

Considering that, here's my first question: For the sake of security, should the insert_data(parameter string) be encoded, even though it is posted and not actually passed along as parameters in an actual URL?

Below is the PHP to which the AJAX request posts the data. In the script I'm trying to sanitize the data before inserting it.

Earlier today I read on SO that htmlspecialchars() and prepared statements should be sufficient, and that there isn't much else one can do, when it comes to sanitizing input. But I figure I might as well try to do everything I can.

$name = trim(strtolower(htmlspecialchars($_POST["name"])));
$email = trim(strtolower(filter_var($_POST["email"], FILTER_SANITIZE_EMAIL)));
$amount = trim(filter_var($_POST["amount"], FILTER_SANITIZE_NUMBER_INT));
$sld = trim(strtolower(htmlspecialchars($_POST["sld"])));
$tld = trim(strtolower(htmlspecialchars($_POST["tld"])));

I also read earlier that FILTER_SANITIZE_MAGIC_QUOTES is now deprecated, even though there's no mention of this at all in the documentation. Because of this, I'm wondering whether any of the following filters are also depcrecated?

  • FILTER_SANITIZE_EMAIL
  • FILTER_SANITIZE_NUMBER_INT
  • FILTER_SANITIZE_SPECIAL_CHARS
  • FILTER_SANITIZE_FULL_SPECIAL_CHARS
  • FILTER_SANITIZE_STRING

And my last question is, if none of the filters above are deprecated, which of the last three filters should I be using for $name, $sld, and $tld, which should be basic ASCII strings? They all seem so similar to one another...

Thanks

oldboy
  • 5,729
  • 6
  • 38
  • 86
  • Can you give us the link where you read that `FILTER_SANITIZE_MAGIC_QUOTES` is deprecated? Just tested it in PHP 7 and it works. – KIKO Software Jul 29 '19 at 07:05
  • @KIKOSoftware ill try to find it. give me a second – oldboy Jul 29 '19 at 07:07
  • @KIKOSoftware https://stackoverflow.com/a/130323/7543162 – oldboy Jul 29 '19 at 07:09
  • That answer says that *magic quotes* are deprecated. The *filter* is still available if you need it to deal with the aftermath of magic quotes even though they've been dead for years. – deceze Jul 29 '19 at 07:10
  • @deceze yeah i know it says magic quotes are deprecated. *but the documentation makes zero mention of its deprecation* which makes me think other `FILTERS` could also be deprecated and we would never know – oldboy Jul 29 '19 at 07:17
  • https://www.php.net/manual/en/security.magicquotes.php – deceze Jul 29 '19 at 07:20
  • @deceze just strange because [this page](https://www.php.net/manual/en/filter.filters.sanitize.php) makes no mention of it. you think they would make a note of its deprecation there too – oldboy Jul 29 '19 at 07:22

1 Answers1

1
`table=offers&name=${children[0].value.toLowerCase()}...`

There's a problem here. If any of the values contain a & and/or =, the meaning of this query string will be altered, and at the very least you'll lose information. You need to encodeURIComponent each individual value before placing it into a query string, e.g.:

`table=offers&name=${encodeURIComponent(children[0].value.toLowerCase())}...`

You don't need to encode anything for sending it over HTTP. The TL;DR is that you must use HTTPS if you're interested in hiding the information from 3rd parties, there's no sensible way around this.

$name = trim(strtolower(htmlspecialchars($_POST["name"])));

Just… don't. Don't HTML-encode your values for storage. What if you send this value in a plaintext email, or use it in some other non-HTML context? It'll contain HTML entities. That's how things like "Dear Mr. O&quot;Connor" happen.

Just store the plain values as is in the database. You may want to validate them, e.g. check that an email looks like an email, but you shouldn't alter the values unless you have a very good reason to.

Use prepared statements to protect against SQL injection. If you ever output those values again somewhere, encode/escape them properly at that point; e.g. htmlspecialchars them when outputting them into HTML.

See:

deceze
  • 510,633
  • 85
  • 743
  • 889
  • ahh good catch. i would obvs then need to decode the values once theyre posted to the PHP file via the AJAX request, correct? i wasnt going to insert them as encoded values. id just like to do everything in my power to prevent SQL injection and XSS. ill give those links a read – oldboy Jul 29 '19 at 07:13
  • any idea about the `FILTERS`?? my big concern is that somebody can obvs call `insert_data()` with whatever parameter they can imagine, which would then be posted to the PHP file. – oldboy Jul 29 '19 at 07:15
  • No, you don't need to decode them on the PHP side. If you use `$_POST`, they're already parsed and appropriately decoded by PHP. You'd only need to worry about this if you were manually parsing that query string in PHP, which you're not. Generally, more isn't better when it comes to encoding/escaping. Doing it properly for the right context ***once*** is enough. URL-encode for embedding values in URL query strings. HTML encode for HTML. SQL-escape for SQL; or better use *prepared statements* and don't worry about escaping at all. – deceze Jul 29 '19 at 07:15
  • And yes, anyone can make any arbitrary HTTP request to your server at any time. Nothing you can do about that. That's why you ***validate*** the received data on the server before doing anything with it. – deceze Jul 29 '19 at 07:20
  • ok cool. i am using prepared statements, but shouldnt i `FILTER` the data for safe measure? or i guess if i validate the data, there really wouldnt be any need for `FILTERING` it, right? – oldboy Jul 29 '19 at 07:21
  • There's *validation* and there's *sanitisation*. Validation tells you whether data conforms to your expectations, and allows you to make your own decision what to do if it doesn't (e.g. reject the entire request if an email doesn't look like an email). Sanitisation silently alters data to make it conform to certain rules. IMO there's very little use for that, as you'll just lose data without knowing it. – deceze Jul 29 '19 at 07:24
  • ok thats what i gathered from your prior comment, but wanted clarification. thanks. doesnt validation get pretty complex when, say, properly validating an email address? ive read that if you wanted to validate an email address absolutely, the regex expression would take up an entire page. – oldboy Jul 29 '19 at 07:29
  • will do. thanks. also notably, the `FILTER_SANITIZE_NUMBER_INT` is basically validation and not sanitization. – oldboy Jul 29 '19 at 07:35
  • Uhm, no. The description says *"Remove all characters except digits, plus and minus sign."*. That's not validation, that's *making everything into a number even if it isn't.* – deceze Jul 29 '19 at 07:37
  • but thats precisely my use case. im looking for numbers only that are not abbreviated or exponents or anything like that... so its basically validation – oldboy Jul 29 '19 at 07:42
  • What if the input is *"fnargle4bl0g1s"*? I would rather *reject* that as nonsense than silently turn it into `401`… – deceze Jul 29 '19 at 07:43
  • i guess it would be ideal to reject anything that isnt purely a number since if its not purely a number then theyre obvs trying to break something or break in. it should be a piece of cake to validate that value too – oldboy Jul 29 '19 at 07:49