http://codereview.appspot.com/2206045/diff/21001/src/com/google/caja/cajita-module.js
File src/com/google/caja/cajita-module.js (right):

http://codereview.appspot.com/2206045/diff/21001/src/com/google/caja/cajita-module.js#newcode195
src/com/google/caja/cajita-module.js:195: var jsonpCallbackCount = 0;
On 2010/10/08 17:37:49, jasvir wrote:
Please move this closer to its single place of use

Done.

http://codereview.appspot.com/2206045/diff/21001/src/com/google/caja/service/AbstractCajolingHandler.java
File src/com/google/caja/service/AbstractCajolingHandler.java (right):

http://codereview.appspot.com/2206045/diff/21001/src/com/google/caja/service/AbstractCajolingHandler.java#newcode172
src/com/google/caja/service/AbstractCajolingHandler.java:172:
output.append(jsonpCallback + "(");
On 2010/10/08 17:37:49, jasvir wrote:
This is vulnerable - jsonpCallback is never checked to be just a
function
identifier and can be arbitrary js.

That is the intention. We are emitting un-cajoled code to a client who
wishes to make their entire page vulnerable to our service. Our only
responsibility is to assure maximum fidelity to whatever it is they give
us. Parsing the callback for correctness would not meaningfully reduce
the attack surface, anyway, because the result would *still* be a piece
of code that calls an arbitrary top-level function which, in the "wrong"
hands, is already dangerous enough.

http://codereview.appspot.com/2206045/diff/21001/src/com/google/caja/service/CajaArguments.java
File src/com/google/caja/service/CajaArguments.java (right):

http://codereview.appspot.com/2206045/diff/21001/src/com/google/caja/service/CajaArguments.java#newcode40
src/com/google/caja/service/CajaArguments.java:40:
JSONP_CALLBACK("jsonp-callback"),
On 2010/10/08 17:37:49, jasvir wrote:
We can call this whatever we like but it might be worthwhile calling
is
"callback" to be compatible with other gdata feeds..

Done.

http://codereview.appspot.com/2206045/diff/21001/src/com/google/caja/service/CajaArguments.java#newcode41
src/com/google/caja/service/CajaArguments.java:41:
On 2010/10/08 17:37:49, jasvir wrote:
In this or in a separate CL, alt=json and alt=json-in-script as
alternatives to
input-mime-type.

I assume you mean as an alternative to the now-defunct
"output-mime-type"? But we *always* produce JSON now....

http://codereview.appspot.com/2206045/diff/21001/src/com/google/caja/service/CajolingServlet.java
File src/com/google/caja/service/CajolingServlet.java (right):

http://codereview.appspot.com/2206045/diff/21001/src/com/google/caja/service/CajolingServlet.java#newcode153
src/com/google/caja/service/CajolingServlet.java:153:
resp.setStatus(HttpServletResponse.SC_OK);
On 2010/10/08 17:37:49, jasvir wrote:
Ok.  Response is always ok if the cajoler ran and output json contains
the
needed errors.

Done.

http://codereview.appspot.com/2206045/

Reply via email to