Escape attempts, date sauce and half-truths
Here are some solutions for the Java puzzles I posted a few weeks ago.
SQL escaping
The first code snippet was this, from Apache Commons:
public static String escapeSql(String str) {
if (str == null) {
return null;
}
return StringUtils.replace(str, "'", "''");
}
It is presumably intended to be used like this:
executeQuery("select * from foo where bar = '" + escapeSql(query) + "'");
Here's Reinier's explanation:
rant #1: Proper style. null is just not going to work in the above example, and it cannot be fixed: Because the caller is responsible for adding the quotes, you end up with a quoted 'null' either way - whether you return the string null (sans quotes) or the actual null value, which gets translated to the string null when concatenating strings.For completeness: you can do SQL injection through the above function by using backslash-quote to end the string. The function expands it to backslash-quote-quote, and Postgres interprets the first backslash-quote as a single quote, leaving the final quote to end the string.Add to that the disastrous side-effect of forgetting to manually add the quotes, security wise, and the story is complete:
Letting the caller add the outside quotes is Wrong, with a very large capital W.
rant #2: Security.
I don't know how to circumvent that escapeSql method. That's the point. I am missing details. Maybe some intermediate layer somewhere tries to be clever and attempts to translate certain unicode characters down to the nearest ASCII equivalent, allowing a character that java does not identify as a quote end up as a quote ANYWAY.
Maybe the database parser has an internal escaping device, like java's own backslash u.
Maybe inserting 'oogieboogieboola' into the string mysteriously accesses a little known backdoor.
The point is this: It depends on the underlying dbase AND version. No way this particular bit of code is intended to be married that explicitly to the database.
This kind of thing is why PHP has awkwardly named functions like pg_sql_escape and why java has the PreparedStatement.
You -always- use them. Even if for all mainstream database engines, all versions, that escapeSql method can be engineered and proven to work reliably... it would still be Wrong.
Date format
I screwed this one up a bit. Reinier found these two bugs that weren't supposed to be there:
- The actual ISO 8601 date format actually has dashes between the fields: 2006-08-19T14:50:32Z.
- I forgot to make the conversion function
static.
- The fact that it claims to make an UTC date, but is actually using the local time zone.
- Incredibly, the
SimpleDateFormatclass has internal state, so it is not safe to share it across threads. Your choice: create a new one each time, synchronize access to it, or use a ThreadLocal.
import java.text.*;
import java.util.*;
public class Dates {
public static void main(String[] s) {
final Set<String> dates = Collections.synchronizedSet(new HashSet<String>());
final DateFormat formatter = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss");
for (final int i : new int[] {2,10}) {
Runnable r = new Runnable() {
public void run() {
Date date = new Date(i+100, i-1, i, i, i, i);
while(true) {
String s = formatter.format(date);
if(dates.add(s)) System.out.println(s);
}
}
};
new Thread(r).start();
}
}
}
Here's the output I got:
2010-10-10T10:10:10 // this is correct 2002-02-02T02:02:02 // this too 2010-10-10T10:02:02 // these are slightly mixed up 2002-10-10T10:10:10 ... 2002-02-02T09:02:02 2010-10-19T14:00:28 // use these to see how the implementation 2002-01-23T22:11:43 2002-02-02T03:10:10 ... 2010-10-10T10:11:43 Exception in thread "Thread-0" java.lang.ArrayIndexOutOfBoundsException: -94 at sun.util.calendar.BaseCalendar.getCalendarDateFromFixedDate(BaseCalendar.java:436) at java.util.GregorianCalendar.computeFields(GregorianCalendar.java:2055) at java.util.GregorianCalendar.computeFields(GregorianCalendar.java:1970) at java.util.Calendar.setTimeInMillis(Calendar.java:1066) at java.util.Calendar.setTime(Calendar.java:1032) at java.text.SimpleDateFormat.format(SimpleDateFormat.java:785) at java.text.SimpleDateFormat.format(SimpleDateFormat.java:778) at java.text.DateFormat.format(DateFormat.java:314) at Dates$1.run(Dates.java:14) at java.lang.Thread.run(Thread.java:613)
Booleans
This was a tricky one. A little-known fact about Java is that booleans aren't really limited to false and true.
The code uses the expression
if(newVal == true)
throw new SecurityException("You can't set the flag!");
this.overrideEnabled = newVal;
The problem is the "==true" which is compiled as "==1" - if the user passes a value that is neither false nor true, the flag will be set to that value. When it is later tested without ==true…
if(overrideEnabled) { ... }
… anything non-zero will be interpreted as true, and access is granted.
The fix is simply to remove the "ridiculously superfluous" == true.
Here's a test class:
public class BoolBug {
static boolean override;
public static void setOverride(boolean b) {
if (b == true) throw new SecurityException();
override = b;
}
public static void main(String[] s) {
setOverride( MagicCast.toBool(2) );
System.out.println("The flag is "+override);
}
}
The int-to-boolean casting operation is not expressible as Java source code; here's a pre-compiled MagicCast class.
You can build it using jasmin (the class file assembler) -- the method implementation is just "iload 0; ireturn". Or you can do it the lazy way: compile a "public int toBool(int i) { return i; }" and change the method signature in the class file from (I)I to (I)Z.