commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Groß (JIRA) <j...@apache.org>
Subject [jira] [Commented] (IMAGING-164) Possible dereferencing of null pointer in IcoImageParser::writeImage
Date Sun, 15 Feb 2015 09:00:19 GMT

    [ https://issues.apache.org/jira/browse/IMAGING-164?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14321881#comment-14321881
] 

Michael Groß commented on IMAGING-164:
--------------------------------------

There is no bug. A dereferencing of null pointer can not happen here, ther is just confusing
code.
If the variable palette is null, then bitCount is > 8:
{noformat}
if (palette == null) {
            if (hasTransparency) {
                bitCount = 32;
            } else {
                bitCount = 24;
            }
{noformat}
When accessing the variable palette later, bitCount must have a value <= 8:
{noformat}
if (bitCount < 8) {
        ....
        final int index = palette.getPaletteIndex(rgb);
        ...
    } else if (bitCount == 8) {
        ....
        final int index = palette.getPaletteIndex(rgb);
        ...
    } else if (bitCount == 24) {  //no access to palette
        bos.write(0xff & argb);
        bos.write(0xff & (argb >> 8));
        bos.write(0xff & (argb >> 16));
    } else if (bitCount == 32) {   //no access to palette
        bos.write(0xff & argb);
        bos.write(0xff & (argb >> 8));
        bos.write(0xff & (argb >> 16));
        bos.write(0xff & (argb >> 24));
    }
{noformat}

> Possible dereferencing of null pointer in IcoImageParser::writeImage
> --------------------------------------------------------------------
>
>                 Key: IMAGING-164
>                 URL: https://issues.apache.org/jira/browse/IMAGING-164
>             Project: Commons Imaging
>          Issue Type: Improvement
>          Components: Format: ICO
>            Reporter: Michael Groß
>             Fix For: Patch Needed
>
>
> org.apache.commons.imaging.formats.ico.IcoImageParser::writeImage(final BufferedImage
src, final OutputStream os, final ImagingParameters params)
> may throw an unexpected NullPointerException because it of the following code:
> {noformat}
> final SimplePalette palette = paletteFactory.makeExactRgbPaletteSimple(src, 256);
> {noformat}
> Then asking if the created palette is null. I will discuss where it comes from below.
For now it is interesting that we set the variable bitCount despite the SimplePalette is null.
Currently this makes no sense because the code will throw a NullPointerException later if
SimplePalette is null.
> {noformat}
> if (palette == null) {
>             if (hasTransparency) {
>                 bitCount = 32;
>             } else {
>                 bitCount = 24;
>             }
> {noformat}
> In the later for-loop we try to call *getPaletteIndex(rgb)* on the SimplePalette instance.
If it contains null, we'll get a NullPointerException here.
> {noformat}
> for (int y = src.getHeight() - 1; y >= 0; y--) {
>             for (int x = 0; x < src.getWidth(); x++) {
>                 final int argb = src.getRGB(x, y);
>                 if (bitCount < 8) {
>                     final int rgb = 0xffffff & argb;
>                     final int index = palette.getPaletteIndex(rgb); // possible NullPointerException
>                    ...
>                 } else if (bitCount == 8) {
>                     final int rgb = 0xffffff & argb;
>                     final int index = palette.getPaletteIndex(rgb);  // possible NullPointerException
> {noformat}
> Why can SimplePalette be null? It comes from PaletteFactory::makeExactRgbPaletteSimple(final
BufferedImage src, final int max). As it's javadoc says it will "fails by returning null if
there are more than max colors necessary":
> {noformat}
> if (rgbs.add(rgb) && rgbs.size() > max) {
>                     return null;
>                 }
> {noformat}
> My first idea goes to throw a RunTimeException rather than returning null. But one has
to check if there are cases where the return of null triggers some error handling i.e. increasing
the number of colors or creating a different type of object.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message