Sam Varshavchik wrote: > Alessandro Vesely writes: > >> >> * use mysql escape function also in a number of other places; the >> MySQL team took years to get it straight... > > Well, I don't think they got it right. There's no bounds checking in > mysql_real_escape_string! The documentation claims you just need to > provide enough room at least twice as long as the string length, but > then there are also some vague comments regarding the interaction of > this function with the locale's character set, which leaves me with a > somewhat uneasy feeling.
Since they require 2*length+1, I assume they check that bound. I don't know the details of the implementation, but doubling seems quite enough. Even if mysql_real_escape_string() cannot fail, its output will eventually be parsed using some other function which is supposedly aware about what the former might have done. > This is too dangerous, and opens the possibility of exploitable buffer > overflow -- and it looks to me like your patch takes into account the > additional buffer requirements, I don't see anything in the patch that > allocates sufficient memory for the potentially escaped string. I juxtaposed some "2*" to the lengths of fields that need escaping, possibly checking that the "+1" was taken into account already. > This is much more tricky that it seems at first. It's possible to do > this right, but it will require much more elbow grease. Besides character sets, there is the NO_BACKSLASH_ESCAPES setting. A custom escaping function should be aware of the details of MySQL client and server versions, and collect relevant settings from the server. If the MySQL team cannot afford enough elbow grease for that, why should we think we can? The alternative is to be overly restrictive in the input we accept. I must admit I wrongfully blamed the current get_localpart(). (That's because I managed to get some quotes through. However, I tried again and I cannot reproduce that on my running system. Apologies for my oversight.) Yet, one reason why get_localpart() is broken is that it doesn't allow crappy mailboxes such as "with \"the\" quotes"@example.com which are allowed with prepared statements. Allowed mailboxes should also be able to change passwords and enumerate, which can be achieved by running prepared statements for them too. On the other hand, we cannot guarantee that prepared statements work in *all* cases. Thus, we still need direct queries alongside. However, they won't be perfectly equivalent because of incompatible escaping. Thus, it may seem odd to fuzzily do one or the other kind of query, although the decision is ultimately based upon user's configuration. What do you think? ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone _______________________________________________ courier-users mailing list [email protected] Unsubscribe: https://lists.sourceforge.net/lists/listinfo/courier-users
