code review

[rulezbreaker]

Can anyone review the following code snippet "security wise" and advice me on whether it is vulnerable or not:

bool Processnput::onDecode(Stream* st, Bitmap* bm,
Bitmap::Config pref, Mode mode)
...
int width = readByte(buf, 6 + i*16);
int height = readByte(buf, 7 + i*16);
if (st->read((void*)buf, length) != length) {
return false;
}
int offset = read4Bytes(buf, 18 + i*16);
int bitCount = read2Bytes(buf, offset+14);
switch (bitCount)
{
case 1:
case 4:
c = Bitmap::kIndex8_Config;
break;
case 8:
case 24:
case 32:
c = Bitmap::kARGB_8888_Config;
break;
default:
RETURN_ERROR(("Image with %d not supported\n", bitCount));
continue;
}
...
}

Thanks

[kbw]

It doesn't help if you remove declarations.

[rulezbreaker]

Thanks,

Here is the rest of the code if it helps.
http://code.google.com/p/skia/source/browse/trunk/src/images/SkImageDecoder_libico.cpp?r=545

[kbw]

I don't know enough about bitmap processing to comment on the algorihm; that aside, the code looks good to me.

I assume the pointer from SkAutoMalloc is delete when it goes out of scope. If that's the case, you might want to consider using it for color so you don't have to worry about releasing it on the various exit points.