Medallia Blog

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.

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.

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.

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 original two bugs were

• The fact that it claims to make an UTC date, but is actually using the local time zone.
• Incredibly, the SimpleDateFormat class 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.

Here's a test program. It tries to format the two dates 2/2/02 02:02:02 and 10/10/10 10:10:10 using a shared formatter, and prints out all the different results it gives.

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.

Internally, they are just integers, and if you use a value other than 0 or 1, the behavior is undefined. This is not undefined as in C, where the standard explicitly states that it will not define the behavior; it is literally not mentioned in the spec.

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.