Having fought with image rewriting on our platform a little little bit (some of my colleagues have torn their hair out over it), I know that this sounds like a good idea. It only sounds like one. There is no Java image library (JAI, AWT, JNI to ImageMagick, you name it) than can resist a determined attacker or a dumb user wanting to take your JVM down. End of story.
Please, at least, add a switch so that one can turn it off. Thanks. At least I don't plan to let our Shindig installs to go near there and if I would rewrite images, I would use our painstakely tuned image processors. Which don't use Java code. I would assume that at least Paul has similar war stories to share. Ciao Henning On Wed, Jan 21, 2009 at 15:52, Louis Ryan <lr...@google.com> wrote: > So after a fair bit of research and learning about the evils of the Java > ImageIO library I plan on re-writing the code using Sanselan ( > http://incubator.apache.org/sanselan/site/index.html) which is an Apache > incubator project for image handling in pure Java. I'm not aware of any > issues of one incubator project depending on another incubator project (and > having such a rule would seem very odd) > > On Fri, Dec 12, 2008 at 3:15 PM, Brian Eaton <bea...@google.com> wrote: > >> This significantly increases the security risk to Shindig deployments. >> We definitely should not turn this on by default, and I'm not sure we >> should do it at all. >> >> The problem is that most Java image handling is done in native code >> for performance reasons, and that image processing has historically >> been a source of critical vulnerabilities in the JRE: >> >> http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-0243 >> http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-2789 >> >> If a shindig deployment does turn this on, they need to be prepared to >> patch their JRE on very short notice. That's not a realistic option >> for most java shops. >> >> I'm going to look around for alternative implementations that are >> easier to make secure. >> >> Cheers, >> Brian >> >> On Fri, Dec 12, 2008 at 2:43 PM, <louiscr...@gmail.com> wrote: >> > Reviewers: shindig-dev, >> > >> > Description: >> > This code is the basis for integrating image rewriting into the Shindig >> > proxy pipeline. This code will try to make reasonable assumptions about >> > what target image format is most suitable and then serialize to that >> > format. If the content is smaller than the original then it is used. A >> > fairly high degree of configurability is added and protections to ensure >> > that rogue images cannot create exceesively large memory burdens on the >> > VM >> > >> > For sample traffic seen on Orkut this kind of rewriting can yield pretty >> > substantial reductions in image size. Ill post more thorough results >> > later. >> > >> > Details >> > GIF - We dont rewrite animated GIFs though we could it is an expensive >> > process doing frame to frame differencing. We also dont rewrite >> > transparent GIF currently though we could later with a reasonable >> > AlphaImageLoader solution for IE6 >> > >> > PNG - Most PNG writers dont do a good job stripping metadata or >> > minimizing palette and bit depth, or they use palettes when the image is >> > small. This will fix most of this. We also attempt to rewrite to JPEG if >> > the image is opaque and only if it yields a significant improvement ( > >> > 20%) over an efficient PNG. JPEG should only provide significant >> > improvement if the image is photographic in nature. >> > >> > JPEG - We strip metadata and allow for configurable compression level. >> > >> > Please review this at http://codereview.appspot.com/10841 >> > >> > Affected files: >> > java/common/conf/shindig.properties >> > >> >> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BaseOptimizer.java >> > >> >> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizer.java >> > >> >> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/ImageRewriter.java >> > >> >> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/ImageUtils.java >> > >> >> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JPEGOptimizer.java >> > >> >> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/OptimizerConfig.java >> > >> >> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/PNGOptimizer.java >> > >> >> java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/BaseOptimizerTest.java >> > >> >> java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizerTest.java >> > >> >> java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/ImageRewriterTest.java >> > >> >> java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/JPEGOptimizerTest.java >> > >> >> java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/PNGOptimizerTest.java >> > >> >> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/animated.gif >> > >> >> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/inefficient.png >> > >> >> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/large.gif >> > >> >> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/large.jpg >> > >> >> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/rgbawithnoalpha.png >> > >> >> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/small.jpg >> > >> >> java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/unanimated.gif >> > pom.xml >> > >> > >> > >> >