On Fri, Dec 12, 2008 at 3:22 PM, Louis Ryan <lr...@google.com> wrote:
> Actually the default image readers provided by the ImageIO library I've > been > using are Java only. There are native versions in some VMs so I agree that > we need to block those known to have issues. I would be interested to see > how secure this is in OpenJDK. Blocking seems unnecessary. As long as there's an option to disable the rewriting people that don't have a secure library can just disable the feature. > > > 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 > > > > > > > > > > > >