|
Ryouga Inverse posted:The flaw is in your thought process. "Well, I don't know of any attacks that this doesn't prevent against, so this level of security is okay, I'll just use that." Why would you even want to think about that? Just code it the way that's not vulnerable to that class of attack AT ALL and go about your day. I challenge you to construct for me a sample query where prepared statements are any more secure than properly-used mysql_real_escape_string().
|
# ? Jan 8, 2009 01:13 |
|
|
# ? May 16, 2024 17:27 |
|
Atom posted:I challenge you to construct for me a sample query where prepared statements are any more secure than properly-used mysql_real_escape_string(). I challenge you to prove to me that mysql_real_escape_string() takes care of absolutely every possible SQL injection attack, including ones that may be discovered later.
|
# ? Jan 8, 2009 01:21 |
|
By "properly-used" I guess you mean that they're used comprehensively, in which case there is no difference - but that's not what I think Ryouga Inverse was getting at. If you always use escape_string() then there's a risk you might forget to, and that risk isn't possible with prepared statements. However that's beside the point, because I think Ryouga Inverse was referring to mysql_escape_string() vs mysql_real_escape_string(), not escape_string() vs prepared statements.
|
# ? Jan 8, 2009 01:22 |
|
Ryouga Inverse posted:I challenge you to prove to me that mysql_real_escape_string() takes care of absolutely every possible SQL injection attack, including ones that may be discovered later. mysql_real_escape_string() is guaranteed by the MySQL API documentation to be 100% correct. The same level of dignity it gives to its prepared statements, no more, no less. That's why the burden of proof is on you to prove that one method is any more secure than the other.
|
# ? Jan 8, 2009 01:28 |
|
The point is that with using prepared statements you don't need to remember to wrap everything with mysql_real_escape_string(). It also makes your code easier to read when it's not filled with the same stupidly_long_function_name_over_and_over(). If you choose to escape manually, you're going to forget eventually. Will it be exploitable? Depends on how often you forget and where... There's no reason not to use prepared statements. geetee fucked around with this message at 01:38 on Jan 8, 2009 |
# ? Jan 8, 2009 01:34 |
|
geetee posted:The point is that with using prepared statements you don't need to remember to wrap everything with mysql_real_escape_string(). This is not the argument. Especially when prepared statements have an additional overhead of an additional statement sent to MySQL, there is no reason it should be preferred over the traditional method when the traditional method is used properly. Edit: Situations that benefit from being able to use the same prepared statement twice and skip the parse/stats phase of the query the second time are excluded.
|
# ? Jan 8, 2009 01:49 |
|
What is the argument then? Is it mysql_escape_string() vs mysql_real_escape_string(), which is what zergstain was asking about? To reiterate what Ryouga Inverse said, a flaw was found with mes() whereas none has been found with mres(), so use mres(). Just because zergstain couldn't find any exploit code that used mes() isn't a good excuse to keep using it. If the argument is prepared vs thorough use of mres(), then I don't think the choice is so clear cut. Prepared statements are free of injection risks and may be processed faster since the app doesn't have to plod through a potentially large string to copy and escape it, and the DB server doesn't have to spend time parsing the string to unescape it. But the downside is that prepared statements can be a little harder to read (since it's necessary to read 2 areas of code simultaneously to ensure that each replacement token is correctly matched up with the variable that will be replacing it), and can they can be difficult to use in some dynamic SQL situations. Use of mres() can be easier and more legible in these situations, but runs the risk of the coder forgetting to use it.
|
# ? Jan 8, 2009 02:21 |
|
Hell, if it were my personal code I'd have switched over to mres() sometime in 2003 or whenever it was introduced. Assuming I'd been doing PHP for that long. But this is company code, so I don't really want to just go around loving with poo poo. I'll go ahead and mention it tomorrow, though the work they actually want me to do will be a priority, and I'm fairly sure everything is latin 1 anyway. I will admit that I was beginning to wonder about the real world risks of using addslashes() since mysql_[real_]escape_string() takes much longer to type.
|
# ? Jan 8, 2009 02:41 |
|
zergstain posted:and I'm fairly sure everything is latin 1 anyway.
|
# ? Jan 8, 2009 02:53 |
|
minato posted:If you're using mes() or mres() then you're probably escaping user input. How can you be sure all user input is Latin 1? Well, what happens if somebody submits unicode, but the connection charset is latin 1? Edit: mres() escapes based on the connection charset I'm fairly sure, and I assume that means if somebody submitted some Shift-JIS or something it would just be interpreted as Latin 1, so explain how that is relevant. zergstain fucked around with this message at 03:20 on Jan 8, 2009 |
# ? Jan 8, 2009 03:11 |
|
Well, you're asking whether you should get your bosses to approve moving from mes() to mres(). And you're right that mres() does respect the connection's character set. But mes() doesn't:The PHP manual for mes() posted:This function is identical to mysql_real_escape_string() except that mysql_real_escape_string() takes a connection handler and escapes the string according to the current character set. mysql_escape_string() does not take a connection argument and does not respect the current charset setting.
|
# ? Jan 8, 2009 03:38 |
|
No Safe Word posted:*whoooosh* I guess I would have been more suspicious of a fakepost if it was funnier? I.D.K.
|
# ? Jan 8, 2009 05:36 |
|
minato posted:Well, you're asking whether you should get your bosses to approve moving from mes() to mres(). And you're right that mres() does respect the connection's character set. But mes() doesn't: While I'm at it, anybody think I should ask about going through all 3.5 million or so rows in the users table replacing the plaintext passwords with a salted sha1 hash and dealing with the load on the slave servers as they have to deal with all the new data? Any of those people seriously think anyone at work would go for it? AFAIK, they want to be able to see passwords anyway. BTW, this is probably a good example for why you should be careful about reusing your passwords. I suppose at the very least thanks to the insane password requirements on these forums I'd have little chance of doing anything if I ever found a goon in our database. At least the dev environment is PHP 5.2.5 and the live is 5.2.6, unlike that other guy's company. And I get to look at porn. Oh hey: http://ilia.ws/archives/103-mysql_real_escape_string-versus-Prepared-Statements.html mes() would also interpret the sequence wrong and be vulnerable. Assuming I was coding for a Chinese site or something I guess. This gives me the idea to make a wrapper function literally called mres() to reduce typing.
|
# ? Jan 8, 2009 06:32 |
|
Steampunk Mario posted:I guess I would have been more suspicious of a fakepost if it was funnier? I.D.K. Don't get too upset. I know sometimes it's hard to tell the blatantly obvious fake posts apart from the hexadecimal posts. zergstain posted:While I'm at it, anybody think I should ask about going through all 3.5 million or so rows in the users table replacing the plaintext passwords with a salted sha1 hash and dealing with the load on the slave servers as they have to deal with all the new data? geetee fucked around with this message at 06:59 on Jan 8, 2009 |
# ? Jan 8, 2009 06:56 |
|
Ryouga Inverse posted:I challenge you to prove to me that mysql_real_escape_string() takes care of absolutely every possible SQL injection attack, including ones that may be discovered later. I was just visiting this insults page and teapot's quote really resonated with me: teapot posted:No, it merely poisons minds, making programmers incapable of writing high-quality software and developing new ideas. It cripples their model of thinking by pushing them toward the optimal way of programming for severely mis-designed Microsoft products. Destruction of human minds' ability to improve is something that I don't take lightly. Victor fucked around with this message at 07:58 on Jan 8, 2009 |
# ? Jan 8, 2009 07:28 |
|
The mes()/mres() decision shouldn't really be a issue anyway - most well-designed apps would use some form of DB abstraction layer.zergstain posted:AFAIK, they want to be able to see passwords anyway. If that's true, then your bosses are idiots and you're hosed.
|
# ? Jan 8, 2009 08:17 |
|
Victor posted:I was just visiting this insults page I'm really upset that I don't have a section there
|
# ? Jan 8, 2009 08:34 |
|
I guess it's just hard for old people to be truly offensive other than, you know, being old and crufty and smelly with bad breath and only knowing ancient languages that stunt brand new minds. Since that's the kind of person who probably wrote up that site, well, you're not special enough to make it in.
|
# ? Jan 8, 2009 08:36 |
|
Victor posted:Except that Microsoft does not hold the patent on poo poo generation. I would be very surprised if they didn't. Making it illegal to to create mind poisoning software on any Linux except SUSE.
|
# ? Jan 8, 2009 09:02 |
|
Btw mysql_real_escape_string has a long history of vulnerabilities (as does every function ever coded by MySQL AB, funny that...), and the PHP folks are nuts for using a really loving thin wrapper over it generally If you are going to code a database-backed php app just use parameterized queries like every other language/framework out there
|
# ? Jan 8, 2009 09:05 |
|
How do I parameterize on the table from which I am selecting, using this wonderful parameterized functionality like every other language/framework out there?
|
# ? Jan 8, 2009 09:27 |
|
Victor posted:How do I parameterize on the table from which I am selecting, using this wonderful parameterized functionality like every other language/framework out there? http://tinyurl.com/7ex6d8
|
# ? Jan 8, 2009 09:34 |
|
You must have missed that post of passion of mine above. It's ok, it's shorter than what I used to post long ago, so maybe you missed it. Maybe longs posts were good...
|
# ? Jan 8, 2009 10:43 |
|
Victor posted:You must have missed that post of passion of mine above. It's ok, it's shorter than what I used to post long ago, so maybe you missed it. Maybe longs posts were good... Your posts are just as long as they were before, now you just hit "Submit Reply" every few sentences.
|
# ? Jan 8, 2009 10:53 |
|
Apparently PHP 5 supports prepared statements. http://tinyurl.com/7jce4o This mitigates all your strip slashing and quote adding. There is no reason to send a raw query anymore, ever.
|
# ? Jan 8, 2009 11:02 |
|
code:
Just saw this at work, someone should tell this chucklefuck that there's only one sequence point here.
|
# ? Jan 8, 2009 16:11 |
|
PnP Bios posted:Apparently PHP 5 supports prepared statements. Unless your PHP was compiled without mysqli. And does that poo poo work with a 4.0.27 server? Otto Skorzeny posted:Btw mysql_real_escape_string has a long history of vulnerabilities (as does every function ever coded by MySQL AB, funny that...), and the PHP folks are nuts for using a really loving thin wrapper over it generally I'd like to read more about that as I'm curious as to how a function with a fairly simple job can have a "long history of vulnerabilities". A quick google only revealed a multibyte handling issue. minato posted:The mes()/mres() decision shouldn't really be a issue anyway - most well-designed apps would use some form of DB abstraction layer. I did mention that we had a quote method, so all we'd have to do is change that probably.
|
# ? Jan 8, 2009 17:18 |
|
I found this code I wrote a while ago for comparing 2 revisions. This class I made also represents local changes as well as CVS revision, so my idea was to always have local revision to be higher, and then compare by time committed, and if that is equal compare author names. But wtf, that is one big rear end return statement. code:
|
# ? Jan 8, 2009 17:21 |
|
I enjoy that your `wtf' is some trivial syntactic who-cares, along the lines of the people fuming about "return x == true", while you ignore the enjoyable semantic wtf of every instance of that class being `equal' to everything that isn't a CVSRevision (hint: Finally, the implementer must ensure that x.compareTo(y)==0 implies that sgn(x.compareTo(z)) == sgn(y.compareTo(z)), for all z )
|
# ? Jan 8, 2009 17:38 |
|
dazjw posted:I enjoy that your `wtf' is some trivial syntactic who-cares, along the lines of the people fuming about "return x == true", while you ignore the enjoyable semantic wtf of every instance of that class being `equal' to everything that isn't a CVSRevision There will never be any other objects other than CVSRevision coming in there. If an object of other types comes through, this will be an error in my program, and will be caught in another place, since the data structure I use is Vector<CVSRevision> Also what should it return if its not of CVSRevision type? Since its an error anyway, I think 0 is as good as any other return value. My WTF is really because I have trouble reading my own code, which is not good. Probably should never use 3 ternary operators on same line. EDIT: Whoops noticed another horror in same function. Good thing I went back to clean up my project. EDIT2: Lol even another horror in same goddamn function. drat I suck. vvvv I already take care of this in other functions, but its a good suggestion nonetheless. hexadecimal fucked around with this message at 18:33 on Jan 8, 2009 |
# ? Jan 8, 2009 18:21 |
|
hexadecimal posted:There will never be any other objects other than CVSRevision coming in there. If an object of other types comes through, this will be an error in my program, and will be caught in another place, Then when the thing that should never happen actually does happen, you'll have a head start on tracking it down and fixing the bug.
|
# ? Jan 8, 2009 18:31 |
|
hexadecimal posted:Also what should it return if its not of CVSRevision type? Since its an error anyway, I think 0 is as good as any other return value. Yeah, "it's equal" is definitely as good as throwing a ComparisonException or something.
|
# ? Jan 8, 2009 20:21 |
|
Ryouga Inverse posted:Yeah, "it's equal" is definitely as good as throwing a ComparisonException or something. Well, like I said, currently I check for this error in another part of same class. However I agree, that its probably a good idea to generate an exception or some other notification instead of just returning 0.
|
# ? Jan 8, 2009 20:32 |
|
Painless posted:It won't "bitch", but it's quite possible that it will randomly fail! Just use unions drat it. Jeet chirst this is making me so angry. What nubbery.. code:
|
# ? Jan 8, 2009 20:43 |
|
Presto posted:Strictly speaking, this:
|
# ? Jan 8, 2009 20:47 |
|
hexadecimal posted:Well, like I said, currently I check for this error in another part of same class. However I agree, that its probably a good idea to generate an exception or some other notification instead of just returning 0. Why do you even bother checking "o instanceof CVSRevision" if you know only objects of that type will be passed in? The cast would just throw an exception (which is what you want) if you did pass in an object of another type. I also enjoy !(x ^ y), which is equivalent to x == y. quote:Well, like I said, currently I check for this error in another part of same class. However I agree, that its probably a good idea to generate an exception or some other notification instead of just returning 0. Even if you were absolutely required to return some value, 0 is pretty much the worst value you can return. Really, what were you thinking?
|
# ? Jan 8, 2009 21:01 |
|
floWenoL posted:I also enjoy !(x ^ y), which is equivalent to x == y.
|
# ? Jan 8, 2009 21:25 |
|
Can we get a mod to change the thread title to "Coding horrors: hexadecimal's personal pastebin"?
|
# ? Jan 8, 2009 21:33 |
|
Steampunk Mario posted:
Does that even compile? Post-increment returns an rvalue and you can't assign to rvalues.
|
# ? Jan 8, 2009 21:35 |
|
|
# ? May 16, 2024 17:27 |
|
floWenoL posted:Why do you even bother checking "o instanceof CVSRevision" if you know only objects of that type will be passed in? The cast would just throw an exception (which is what you want) if you did pass in an object of another type. Lol yea I know, right? I coded this as fast as I could at the time to meet the dead line and and didn't give it much thought.
|
# ? Jan 8, 2009 21:37 |