January 24th, 2013, 10:40 AM
Review a simple script?
If anyone is free, I don't suppose I could get anyone to review my front-end page and let me know if they've any suggestions or such, and maybe take a peek at the scripting for it and offer any input if needed? The code is to meerly offer listing of bills for the selected month. Final edit was adding in the ability to safely select bills from only 12 most recent months, and ignore older ones. Thanks for any help.
January 24th, 2013, 11:15 AM
$month = $_POST["mo"];
opens yourself up to SQL injection
$invoices = mysqli_query($con,"SELECT * FROM `invoices` WHERE MONTH(due) = " . $month . " AND YEAR(due) = " . $year . " ORDER BY `due` ASC;");
what's wrong with
echo "....<td align=\"center\">" . $paid . "</td>....";
(cleaner looking code)
echo "....<td align='center'>" . $paid . "</td>....";
January 25th, 2013, 08:27 AM
Thanks Northie. I'll toss some cleanup in there for input.
January 25th, 2013, 09:01 AM
"cleanup" in what sense? In your code, you use the HTML escaping function for the database input. That doesn't work, because HTML and SQL are completely different languages with a different syntax.
So "cleanup" isn't really a good name for a function, because there is no universal way to make input safe. It depends on the language and sometimes even the context.
It's also problematic that you only escape certain values. This is extremely error-prone, because you can easily forget values or forget to add the escaping after you've changed something. From taking a glance at the code, I really couldn't tell if it's actually secure.
So I'd use a different approach:
- For dynamic database queries, always use prepared statements. Isn't that the very reason to switch to MySQLi?
- For HTML stuff, always use htmlentities() or rather htmlspecialchars(). Escape any value, regardless of whether it's actually "dangerous" at this point of time.
January 25th, 2013, 09:55 AM
Well, I actually removed my htmlentities function to do my cleanup, and figured since it was a simple reference to the input, I made a shorthand If/Then to check if it is numeric. As far as the prepared statements, yes, I do need to get myself updated, but that will come in time. So, from a drop-down, the online input to be submitted should be a integer 1-12. This should avoid other input anyway, I would assume.