Submitting form via Ajax and inserting into DB

Hi,

I have a comment form with two fields (Name, Comment) and I have the following setup. When the user submits the form, it will be stored in the DB. My question is if I can make it more secure, if so, how?

JS:

var name = document.getElementById('name').value;
var comment = document.getElementById('comment').value;

var xhr = new XMLHttpRequest();
xhr.onreadystatechange = function () {
	if (xhr.readyState == 4) {
		/* Comment sent */
	}
};
xhr.open('POST', '/comment.php', true);
xhr.setRequestHeader('Content-type', 'application/x-www-form-urlencoded');
xhr.send(name='+name+'&comment='+comment);

comment.php

<?php
if ($_SERVER['REQUEST_METHOD'] == 'POST') {
	if ($_POST['name'] && $_POST['comment']) {
		$r = DB::$db->prepare("INSERT INTO comments (comment_name, comment_body) VALUES (?, ?)");
		$r->bind_param('ss', $_POST['name'], $_POST['comment']);
		$r->execute();
	}
}
?>

It works fine but I will be happy to know if I can improve its security. Thanks for any ideas.

“secure” in what context? there are different types of security …

PS. you never check in PHP, whether your data exist in the first place.

User submitted values should be escaped:

like this:

xhr.send(name='+ encodeURIComponent(name) +'&comment='+ encodeURIComponent(comment) );

And you could be more strict about checking if $_POST variables are set to avoid notice errors:

if (isset($_POST['name']) && isset($_POST['comment'])) {

if you want to be very precise you could also use is_string() on them because it is possible to receive arrays!

Other than that I’d say the code is technically secure.

1 Like

They should also be validated once they reach the server and before inserting into the database (or is calling the PHP directly with junk entries a billion times going to be acceptable?

Maybe. They could be. If the business requirements call for it.

In my experience in 99% of cases of small and medium sized web sites yes - this is acceptable. Why? Because it almost never happens, it’s extremely rare unless a site becomes very popular so that bots and other bad guys begins to target it. It’s like implementing a very difficult captcha before hardly anyone visits the site. Theoretically, you are right - every input variable should be thoroughly validated, however it takes time to program it and in most cases the time spent is simply not worth it. I might call it premature validation engineering. When the site gets popular enough for that to be a problem we can always tighten validation methods accordingly - and the benefit of doing that not right away is that we know a lot more about what invalid input is really coming and how to tune the validators.

The most important thing is that security of the system is guaranteed and validators are not necessary for that. We might also do some sanitization of the data, for example inserting NUL (0x00) bytes into text columns of some databases produces errors - but still a database error does not need to be a security threat unless the system spits out error messages that reveal too much about the system.

Please supply form details then so I can arrange an automated process to fill your database with junk.

This is the most basic security measure we are talking about here - all other security measures are rarely needed except with really popular sites but this is where security starts. Basic user input validation eliminated 99% of possible security issues including most if not all of the ones that beginners hear about.

99% of web sites can forget about all security measures except user input validation and will be safe. User input needs to be validated for reasons other than security so using it as the main security measure makes sense.

In fact to save me the trouble of setting up an automated process, please just enter the following into the form a trillion times.

Name: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
Comment: zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz

Secure as in no malicious scripts or hack attempts will pass.

Do you mean I shouldn’t check if data exists in PHP (2nd IF statement)? Can you elaborate this please? I do another check in JS (not shown here for brevity).

Thanks for the encodeURIComponent() and isset() tips.

I checked a couple of tutorials but couldn’t find any validation for especially comment/message field. Any specific tips for that? The comment I expect is normal text in a couple of sentences or paragraphs with regular punctuation (a-zA-Z0-9 .,;?!:-()&'"tab,new line etc.). Shall I use a preg_replace to filter out anything else? What approach would you suggest?

Why would I? The thing is there are not many guys like you who are willing to fill my database with junk.

There is no security threat in junk data if you follow proper security practices like data escaping, prepared statements, etc. Of course, some data must follow certain structure for the application to function properly, like email addresses, maybe phone numbers, etc. - they should be validated. But name, surname, address, user comments… ? No need to validate since even junk will not be a security threat.

Dangerous statement - it’s like saying don’t escape data for SQL or don’t use prepared statements, do only input validation. I hope no one will follow such advice. [quote=“felgall, post:6, topic:222530”]
In fact to save me the trouble of setting up an automated process, please just enter the following into the form a trillion times.

Name: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaComment: zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz
[/quote]
You seem to provide very theoretical examples that have little to do with real world. In my experience of many web sites running such things don’t happen at all or maybe happen once a year or even more rarely - and you are talking about a trillion times? Each to his own but I prefer to spend my time on more worthwhile stuff than coding elaborate input validation schemes that will never really prove useful. When they turn out to be important within the life cycle of a web site then I will create them but in the beginning I’ll just pass.

Of course, plain forms on web sites without any validation are prone to junk robot submissions so some measures need to be taken to prevent that. But in this case the OP is sending data with javascript and javascript is already a pretty high barrier for most robots so he is already protected from the majority of them.

Personally, I wouldn’t validate comment field at all and would allow any data - well, maybe I’d just check for some maximum length. If you allow only certain characters then you never know if you cause trouble to people who may legitimately enter a character outside of your allowed range. There are hundreds of thousands of characters in Unicode - will you analyse each of them and decide if you want to put it in your regex? Felgall might think otherwise but I would just leave it :slight_smile:

You don’t think passing it through strip_tags and / or htmlspecialchars / htmlentities would be the minimum?

http://php.net/manual/en/function.strip-tags.php
http://php.net/manual/en/function.htmlspecialchars.php
http://php.net/manual/en/function.htmlentities.php

I guess it depends on how it will be used, but I’m assuming it will be displayed in a page or three.

Normally, I wouldn’t because I don’t want to strip content that might potentially be meaningful. Of course, it depends on the use case but personally I never do it.

We are talking here about storing data - htmlspecialchars / htmlentities are not meant to be used for storing. It is best to store data as is and use htmlspecialchars / htmlentities when displaying on the page. In such a case striptags is not important for security because any code will just be displayed and not executed.

1 Like

As far as security is concerned you can get away with just input validation 99% of the time with no other security measures applied.

Using prepare statements and escaping data for HTML have nothing to do with security.

Escaping data for SQL when you use prepare statements is completely unnecessary as escaping is only needed to identify data when it is jumbled with code and could be misinterpreted as code.

Where code and data are jumbled together you need to escape the VALID data.

Aree you deliberately treying to make others sites vulnerable by telling them NOT to implement the most basic of security measures?

If you apply no other security at all the bare minimum is to validate all user inputs. If you don’t do that then you have no security as anything else without that is a small patch on a huge hole.

Reading this all I can say is I agree to disagree and quit the discussion at this point.

Than you don’t know what security is. Doing everything in your power to prevent a breach of a system is the very definition of security.

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.