Monday, February 9, 2009

Defending against XSS attacks in Freemarker

While Freemarker is quite rich in features, it seems to lack support for programmatically declaring HTML-escaping the default behavior for property access. You need to either add a ?html to every access or wrap every single template into this:


<#escape x as x?html>
... your template code ...
</#escape>


This really has to be done on every file, including those being included such as macro definitions.

Both approaches rely on people remembering to do the right thing, and I don't trust anyone that much, particularly not myself. So instead I decided to add this bit of wrapper code programmatically in the code that loads the template. I took the idea from a posting on the freemarker-user mailing list. Instead of using the normal ClassTemplateLoader, I now do this:


final TemplateLoader templateLoader = new ClassTemplateLoader(this.getClass(), templatePath){
@Override
public Reader getReader(Object templateSource, String encoding) throws IOException {
return new WrappingReader(super.getReader(templateSource, encoding), "<#escape x as x?html>", "");
}
};
configuration.setTemplateLoader(templateLoader);


This uses the following class:


package domain.your.util;

import java.io.IOException;
import java.io.Reader;
import java.util.logging.Level;
import java.util.logging.Logger;

public class WrappingReader extends Reader {

private final Reader originalReader;
private final char[] prologue;
private final char[] epilogue;
private int pos = 0;
private int firstEpilogueChar = -1;
private boolean closed = false;

public WrappingReader(Reader originalReader, char[] prologue, char[] epilogue, Object lock) {
super(lock);
this.originalReader = originalReader;
this.prologue = prologue;
this.epilogue = epilogue;
}

public WrappingReader(Reader originalReader, char[] prologue, char[] epilogue) {
this.originalReader = originalReader;
this.prologue = prologue;
this.epilogue = epilogue;
}

public WrappingReader(Reader originalReader, String prologue, String epilogue, Object lock) {
super(lock);
this.originalReader = originalReader;
this.prologue = prologue.toCharArray();
this.epilogue = epilogue.toCharArray();
}

public WrappingReader(Reader originalReader, String prologue, String epilogue) {
this.originalReader = originalReader;
this.prologue = prologue.toCharArray();
this.epilogue = epilogue.toCharArray();
}

@Override
public int read(char[] cbuf, int off, int len) throws IOException {
if (closed) {
throw new IOException("Reader has been closed already");
}
int oldPos = pos;
Logger.getLogger(getClass().getName()).log(Level.FINE, String.format("Reading %d characters from position %d", len, pos));
if (pos < this.prologue.length) {
final int toCopy = Math.min(this.prologue.length - pos, len);
Logger.getLogger(getClass().getName()).log(Level.FINE, String.format("Copying %d characters from prologue", toCopy));
System.arraycopy(this.prologue, pos, cbuf, off, toCopy);
pos += toCopy;
if (toCopy == len) {
Logger.getLogger(getClass().getName()).log(Level.FINE, "Copied from prologue only");
return len;
}
}
if (firstEpilogueChar == -1) {
final int copiedSoFar = pos - oldPos;
final int read = originalReader.read(cbuf, off + copiedSoFar, len - copiedSoFar);
Logger.getLogger(getClass().getName()).log(Level.FINE, String.format("Got %d characters from delegate", read));
if (read != -1) {
pos += read;
if (pos - oldPos == len) {
Logger.getLogger(getClass().getName()).log(Level.FINE, "We do not reach epilogue");
return len;
}
}
firstEpilogueChar = pos;
}
final int copiedSoFar = pos - oldPos;
final int epiloguePos = pos - firstEpilogueChar;
final int toCopy = Math.min(this.epilogue.length - epiloguePos, len - copiedSoFar);
if((toCopy <= 0) && (copiedSoFar == 0)) {
return -1;
}
Logger.getLogger(getClass().getName()).log(Level.FINE, String.format("Copying %d characters from epilogue", toCopy));
System.arraycopy(this.epilogue, epiloguePos, cbuf, off + copiedSoFar, toCopy);
pos += toCopy;
Logger.getLogger(getClass().getName()).log(Level.FINE, String.format("Copied %d characters, now at position %d", pos-oldPos, pos));
return pos - oldPos;
}

@Override
public void close() throws IOException {
originalReader.close();
closed = true;
}
}


Note that this means that in some cases you might need to escape the escaping, which Freemarker allows with the <#noescape> directive. You also can't use template configuration via the <#ftl> directive anymore, since that would need to be before the <#escape>. Since I never felt the urge to use it, I don't care.

No comments: