Hey Everyone, I'm hoping you can shed some light on this for me...Whenever anyone submits a query on my site with the contact us feature, I don't receive the message. There seems to be some code missing...You guys are way smarter than me, and was hoping you'd help... Thanking you in advance! Here is the code: <?php include_once("header.php"); require_once("config/config.php"); $suc_msg=''; if(isset($_POST['submit'])) { $query= "insert into contact(name,email,contact,query,message)values('".$_POST['name']."', '".$_POST['email']."','".$_POST['contact']."','".$_POST['query']."','".$_POST['description']."')"; mysql_query($query) or die(mysql_error()); $suc_msg='We came across an error. Please e-mail steve@mysite.com and we will get back to you ASAP'; /*echo "<script language='javascript'> alert('Sent Successfully'); window.location.href='contact_us.php'</script>";*/ } ?> <form name="reg" id="reg" method="post"> <table width="800px" border="0" align="center" cellpadding="2" cellspacing="5" class="user-registration"> <tr> <td colspan="2" align="left" style="border-bottom:1px solid #c1272d;font-size:22px;font-family:"Myriad Pro"">Contact Us</td> </tr> <tr> <?php if($suc_msg!=''){?> <td colspan="2" align="left" valign="top" class="successtxt"> <?php echo $suc_msg;?> </td> <?php }?> <td> </td> </tr> <tr> <td> <label>Name:<span style="color:#C1272D";>*</span><label> </td> <td> <input type="text" name="name" placeholder="Enter your Name" id="name" class="out"/> </td> </tr> <tr> <td> <label>E-Mail Address:<span style="color:#C1272D";>*</span></label><br/> </td> <td> <input type="text" name="email" id="email" placeholder="Enter Valid Email Id" class="out"/> </td> </tr> <tr> <td> <label>Contact No.:<span style="color:#C1272D";>*</span></label><br/> </td> <td> <input type="text" name="contact" id="amt" placeholder="Enter Contact Number" class="out"/> </td> </tr> <tr> <td> <label>Subject:<span style="color:#C1272D";>*</span></label><br/> </td> <td> <input type="text" name="query" id="query" placeholder="Enter Subject" class="out"/> </td> </tr> <tr> <td> <label >Your Message Here:<span style="color:#C1272D";>*</span></label><br/> </td> <td><textarea name="description" rows=10 cols=71 placeholder="Enter Message Here!!" class="out1"></textarea> </td> </tr> <tr> <td colspan="2" class="contact_sp"> <input name="submit" type="submit" onclick="return validateTextBoxes('reg');" value="Submit" class="submit-val"/> <input name="reset" type="reset" value="Reset" class="submit-val"/> </td> </tr> </table> </form> <?php include_once("footer.php")?> PHP:
Wow... can we have a link to that form in action? That is so open for SQL-injection it's not even funny... And... where is this message supposed to show up? In your inbox (email), or via a counter telling you you have new messages when you log in to your webpage?
It really seems like you don't know what you're doing in PHP at all, perhaps that is the problem. 1) The variables you are inserting into the database via MySQL need to be set like $var = mysql_real_escape_string($_POST['variable']); otherwise the website is wide open to any type of SQL injection. There are people who go on Google looking for contact forms to find unprotected ones to take down 2) The alert when the message is successfully sent is in a comment block so that part of the code would never be executed 3) Are you looking for the message to be sent to your e-mail? Because in no part of the script will any e-mail be sent to you, if that's what you are trying to accomplish you should use PHP's mail() function 4) You didn't end your if else... statement with a final else { } I don't know if this causes any problems in PHP honestly because I've never done it I always end my statements but regardless that could be causing problems 5) Make sure that in either the header.php or config.php you are connecting to a MySQL database (with correct information) and then selecting a database to insert the data to (make sure the database exists and the name is spelt correctly in the code) 6) You may need a space in between message) and values 7) The table "contact" may not exist You should really be more specific about what isn't working with this code and any errors you are receiving, it would make it a lot easier to help you
Basically, whomever did that script for you, does not know what s/he is doing. There are som many mistakes and potential pitfalls, and besides, the whole thing is set up in a table-layout, which isn't really up to today's standards in any way - it's basically looking like end-of-the-90's code, and that is not a Good Thing(tm). The only thing I need to add/embellish about what Pudge1 said - there are no problems not ending an if/else statement without the else in PHP. However, there are other potential problems in that MySQL-query - for one, I'm pretty sure 'query' is a reserved word, and needs to be put in backticks like this: `query`. Here's an (untested) code example, based on what you've already got: <?php include_once("header.php"); require_once("config/config.php"); $suc_msg = ''; if(isset($_POST['submit'])) { foreach($_POST as $key=>$value) { ${$key} = mysql_real_escape_string($value); } $query = "INSERT INTO contact (`name`,`email`,`contact`,`query`,`message`) VALUES ('$name','$email','$contact','$query','$description')"; $result = mysql_query($query); if (!$result) { $suc_msg = 'We came across an error. Please e-mail <a href="mailto:steve@mysite.com">steve@mysite.com</a> and we will get back to you ASAP'; } else { $to = 'steve@mysite.com'; $subject = $query; $message = $description; $headers = "From: ".$email.""."\r\n"; mail($to,$subject,$message,$headers); } echo "<script language='javascript'> alert('Sent Successfully'); window.location.href='contact_us.php'</script>"; } ?> <form name="reg" id="reg" method="post"> <table width="800px" border="0" align="center" cellpadding="2" cellspacing="5" class="user-registration"> <tr> <td colspan="2" align="left" style="border-bottom:1px solid #c1272d;font-size:22px;font-family:"Myriad Pro"">Contact Us</td> </tr> <tr> <?php if($suc_msg!=''){?> <td colspan="2" align="left" valign="top" class="successtxt"> <?php echo $suc_msg;?> </td> <?php }?> <td> </td> </tr> <tr> <td> <label>Name:<span style="color:#C1272D";>*</span><label> </td> <td> <input type="text" name="name" placeholder="Enter your Name" id="name" class="out"/> </td> </tr> <tr> <td> <label>E-Mail Address:<span style="color:#C1272D";>*</span></label><br/> </td> <td> <input type="text" name="email" id="email" placeholder="Enter Valid Email Id" class="out"/> </td> </tr> <tr> <td> <label>Contact No.:<span style="color:#C1272D";>*</span></label><br/> </td> <td> <input type="text" name="contact" id="amt" placeholder="Enter Contact Number" class="out"/> </td> </tr> <tr> <td> <label>Subject:<span style="color:#C1272D";>*</span></label><br/> </td> <td> <input type="text" name="query" id="query" placeholder="Enter Subject" class="out"/> </td> </tr> <tr> <td> <label >Your Message Here:<span style="color:#C1272D";>*</span></label><br/> </td> <td><textarea name="description" rows=10 cols=71 placeholder="Enter Message Here!!" class="out1"></textarea> </td> </tr> <tr> <td colspan="2" class="contact_sp"> <input name="submit" type="submit" onclick="return validateTextBoxes('reg');" value="Submit" class="submit-val"/> <input name="reset" type="reset" value="Reset" class="submit-val"/> </td> </tr> </table> </form> <?php include_once("footer.php")?> PHP:
And there you have it. A perfect example of why importing user defined variables to the local scope is a bad idea. You just made your code vulnerable to XSS injections. All I have to do is rename one of the fields to "suc_msg" and insert some Javascript. Then, inside the loop, it will override the existing $suc_msg variable and output it later inside the form. And since the $email variable is not filtered at all, the form is vulnerable to header injections, which would allow others to send spam through your server.
Easily fixable - I didn't look to hard on security, but you can just get rid of the suc_msg altogether, and replace the <?php if($suc_msg!=''){?> <td colspan="2" align="left" valign="top" class="successtxt"> <?php echo $suc_msg;?> </td> <?php }?> PHP: with <?php if (!$result) { ?><td colspan="2" align="left" vallign="top" class="successtxt">We came across an error. Please e-mail <a href="mailto:steve@mysite.com">steve@mysite.com</a> and we will get back to you ASAP</td><?php } ?> PHP: As for the potential email-attack, yes, that is vulnerable. Suggested fix: (as long as you're running on PHP 5.2 or later) $email = filter_input(INPUT_POST, 'email', FILTER_VALIDATE_EMAIL); if ($email) { $headers .= "From: $email\r\n"; } PHP: This would replace the $headers already in the above example.
Much better! I'm still not sold on the loop that imports variables, though. Are the few extra keystrokes you're saving worth the risk of making things vulnerable? I'm not saying this in response to this topic, but more in general. What if you prefix your imported variables like this: ${"p_$key"} = mysql_real_escape_string($value); PHP: ... ?
bro i don't understand your code,but i will like to share the one i used with you. I used this a nsu-d dot com,go there and see the contact form if you want. create two script,one contact.php and contacts.php. Add email where you will like the message sent for you to read. In the contact.php add this <form action='contacts.php' method='post'> <div class="box"> <h1>Contact:</h1> <label> <span>Full name</span> <input type="text" class="input_tex" name="name" id="name"/> </label> <label> <span>Email</span> <input type="text" class="input_tex" name="email" id="email"/> </label> <label> <span>Subject</span> <input type="text" class="input_tex" name="contact" id="contact"/> </label> <label> <span>Message</span> <textarea class="message" name="message" id="message"></textarea> <p> <input type="submit" class="butto" value="Submit" name="submit" /> </p> </label> </div> </form> PHP: In the contacts.php add this <?php if(isset($_POST['submit'])) { $to = 'put your website email here' ; //put your email address on which you want to receive the information $subject = 'Hello'; //set the subject of email. $headers = 'MIME-Version: 1.0' . "\r\n"; $headers .= 'Content-type: text/html; charset=iso-8859-1' . "\r\n"; $message = "<table><tr><td>Your Name: </td><td>".$_POST['name']."</td></tr> <tr><td>E-Mail: </td><td>".$_POST['email']."</td></tr> <tr><td>Subject: </td><td>".$_POST['contact']."</td></tr> <tr><td>Message: </td><td>".$_POST['message']."</td> </tr></table>" ; mail($to, $subject, $message, $headers); if(mail){ echo "We've recived your message,we will contact you back soon."; } else { echo "There is a error in your message"; } } ?> PHP:
No no no no! This is just as vulnerable as the original code - no enconding of any values, no check to see if the user is inputting anything malicious. BAD!
I'm not really concerned about such, but then again, I don't use mysql_ - I put those variables into prepared statements via PDO, and as such, it doesn't really matter much if they try to create other variable names - they will simply get chucked out. And, even though someone could make a variable named $_POST['yourehacked'] and put something in there, it would not be used anywhere - it would just be declared as a variable in the loop, but not being used anywhere else, so I don't really see the problem with that.
@PoPSiCLe of-course i know,he should add that by himself,am not suppose to teach him everything. All i know is,that form is working,and inserting messages into database is wrong,as he is doing. All message should go to a email which he wants to receive them.
There's nothing wrong with storing messages in a database, as well as sending them by email. You might want to have the messages available within the page, for an admin-page for instance, where you can use the stored messages to have a kind of CRM - that's just one way it can be useful. Nothing wrong in storing them. And, while your form might be working, it's still full of security holes, blantatly so, which might provide a way for malicious users to for instance, send spam-mail, and ultimately might get your account, or server, banned/blacklisted - which you definitely don't want, it's a pain to get it off those lists.
Well if you're sure no one can override important variables you'll be fine. Just keep in mind that users would also be able to override superglobals such as $_SERVER, $GLOBALS, etc... if the loop is not inside a function or method.
That is true - $GLOBALS are off, but $_SERVER is of course a possible vulnerability, hence I rarely use those for anything apart from maybe getting server-time and such. And mostly, even though users can change POST-variables, and give them names, if they want, that will hardly get them very far, as far as I can understand it. Maybe I'm not thinking deviously enough, but I can't for the life of me find ways they can actually do anything with the page by changing the POST-variables (at least not where I'm using them, as far as I can see).