From f1f19f1fff801c9970af627e050becc2f13f82e7 Mon Sep 17 00:00:00 2001 From: Jun Fang Date: Fri, 9 Oct 2015 13:14:54 +0800 Subject: Fix heap-buffer-overflow in color_sycc_to_rgb It's a bug existing in the conversion from YUV420 to RGB. For YUV 420 format, four pixels have 4 Y but only one U and one V. In some cases, there are odd columns or lines in some images. The pixels on last line or column may have Y but no U or V data. For this case, We shall extend U or V using the data on previous column or line. BUG=497357 R=tsepez@chromium.org Review URL: https://codereview.chromium.org/1342683002 . --- core/src/fxcodec/codec/codec_int.h | 2 + core/src/fxcodec/codec/fx_codec_jpx_opj.cpp | 122 +++++++++++++++-------- core/src/fxcodec/codec/fx_codec_jpx_unittest.cpp | 76 ++++++++++++++ 3 files changed, 160 insertions(+), 40 deletions(-) diff --git a/core/src/fxcodec/codec/codec_int.h b/core/src/fxcodec/codec/codec_int.h index b08dee7422..9f408dd9a2 100644 --- a/core/src/fxcodec/codec/codec_int.h +++ b/core/src/fxcodec/codec/codec_int.h @@ -312,6 +312,8 @@ struct DecodeData { OPJ_SIZE_T offset; }; +void sycc420_to_rgb(opj_image_t* img); + /* Wrappers for C-style callbacks. */ OPJ_SIZE_T opj_read_from_memory(void* p_buffer, OPJ_SIZE_T nb_bytes, diff --git a/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp b/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp index 76d62f763b..616af36b00 100644 --- a/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp +++ b/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp @@ -11,6 +11,7 @@ #include "../../../../third_party/lcms2-2.6/include/lcms2.h" #include "../../../../third_party/libopenjpeg20/openjpeg.h" #include "../../../include/fxcodec/fx_codec.h" +#include "../../../include/fxcrt/fx_safe_types.h" #include "codec_int.h" static void fx_error_callback(const char* msg, void* client_data) { @@ -256,29 +257,58 @@ static void sycc422_to_rgb(opj_image_t* img) { img->comps[1].dy = img->comps[0].dy; img->comps[2].dy = img->comps[0].dy; } -static void sycc420_to_rgb(opj_image_t* img) { - int *d0, *d1, *d2, *r, *g, *b, *nr, *ng, *nb; - const int *y, *cb, *cr, *ny; - int maxw, maxh, max, offset, upb; - int i, j; - i = (int)img->comps[0].prec; - offset = 1 << (i - 1); - upb = (1 << i) - 1; - maxw = (int)img->comps[0].w; - maxh = (int)img->comps[0].h; - max = maxw * maxh; - y = img->comps[0].data; - cb = img->comps[1].data; - cr = img->comps[2].data; - d0 = r = FX_Alloc(int, (size_t)max); - d1 = g = FX_Alloc(int, (size_t)max); - d2 = b = FX_Alloc(int, (size_t)max); - for (i = 0; (OPJ_UINT32)i < (maxh & ~(OPJ_UINT32)1); i += 2) { - ny = y + maxw; - nr = r + maxw; - ng = g + maxw; - nb = b + maxw; - for (j = 0; (OPJ_UINT32)j < (maxw & ~(OPJ_UINT32)1); j += 2) { +static bool sycc420_size_is_valid(OPJ_UINT32 y, OPJ_UINT32 cbcr) { + if (!y || !cbcr) + return false; + + return (cbcr == y / 2) || ((y & 1) && (cbcr == y / 2 + 1)); +} +static bool sycc420_must_extend_cbcr(OPJ_UINT32 y, OPJ_UINT32 cbcr) { + return (y & 1) && (cbcr == y / 2); +} +void sycc420_to_rgb(opj_image_t* img) { + OPJ_UINT32 prec = img->comps[0].prec; + if (!prec) + return; + OPJ_UINT32 offset = 1 << (prec - 1); + OPJ_UINT32 upb = (1 << prec) - 1; + OPJ_UINT32 yw = img->comps[0].w; + OPJ_UINT32 yh = img->comps[0].h; + OPJ_UINT32 cbw = img->comps[1].w; + OPJ_UINT32 cbh = img->comps[1].h; + OPJ_UINT32 crw = img->comps[2].w; + OPJ_UINT32 crh = img->comps[2].h; + if (cbw != crw || cbh != crh) + return; + if (!sycc420_size_is_valid(yw, cbw) || !sycc420_size_is_valid(yh, cbh)) + return; + bool extw = sycc420_must_extend_cbcr(yw, cbw); + bool exth = sycc420_must_extend_cbcr(yh, cbh); + FX_SAFE_DWORD safeSize = yw; + safeSize *= yh; + if (!safeSize.IsValid()) + return; + int* r = FX_Alloc(int, safeSize.ValueOrDie()); + int* g = FX_Alloc(int, safeSize.ValueOrDie()); + int* b = FX_Alloc(int, safeSize.ValueOrDie()); + int* d0 = r; + int* d1 = g; + int* d2 = b; + const int* y = img->comps[0].data; + const int* cb = img->comps[1].data; + const int* cr = img->comps[2].data; + const int* ny = nullptr; + int* nr = nullptr; + int* ng = nullptr; + int* nb = nullptr; + OPJ_UINT32 i = 0; + OPJ_UINT32 j = 0; + for (i = 0; i < (yh & ~(OPJ_UINT32)1); i += 2) { + ny = y + yw; + nr = r + yw; + ng = g + yw; + nb = b + yw; + for (j = 0; j < (yw & ~(OPJ_UINT32)1); j += 2) { sycc_to_rgb(offset, upb, *y, *cb, *cr, r, g, b); ++y; ++r; @@ -302,7 +332,11 @@ static void sycc420_to_rgb(opj_image_t* img) { ++cb; ++cr; } - if (j < maxw) { + if (j < yw) { + if (extw) { + --cb; + --cr; + } sycc_to_rgb(offset, upb, *y, *cb, *cr, r, g, b); ++y; ++r; @@ -316,13 +350,17 @@ static void sycc420_to_rgb(opj_image_t* img) { ++cb; ++cr; } - y += maxw; - r += maxw; - g += maxw; - b += maxw; - } - if (i < maxh) { - for (j = 0; (OPJ_UINT32)j < (maxw & ~(OPJ_UINT32)1); j += 2) { + y += yw; + r += yw; + g += yw; + b += yw; + } + if (i < yh) { + if (exth) { + cb -= cbw; + cr -= crw; + } + for (j = 0; j < (yw & ~(OPJ_UINT32)1); j += 2) { sycc_to_rgb(offset, upb, *y, *cb, *cr, r, g, b); ++y; ++r; @@ -336,7 +374,11 @@ static void sycc420_to_rgb(opj_image_t* img) { ++cb; ++cr; } - if (j < maxw) { + if (j < yw) { + if (extw) { + --cb; + --cr; + } sycc_to_rgb(offset, upb, *y, *cb, *cr, r, g, b); } } @@ -347,14 +389,14 @@ static void sycc420_to_rgb(opj_image_t* img) { img->comps[1].data = d1; FX_Free(img->comps[2].data); img->comps[2].data = d2; - img->comps[1].w = maxw; - img->comps[1].h = maxh; - img->comps[2].w = maxw; - img->comps[2].h = maxh; - img->comps[1].w = (OPJ_UINT32)maxw; - img->comps[1].h = (OPJ_UINT32)maxh; - img->comps[2].w = (OPJ_UINT32)maxw; - img->comps[2].h = (OPJ_UINT32)maxh; + img->comps[1].w = yw; + img->comps[1].h = yh; + img->comps[2].w = yw; + img->comps[2].h = yh; + img->comps[1].w = yw; + img->comps[1].h = yh; + img->comps[2].w = yw; + img->comps[2].h = yh; img->comps[1].dx = img->comps[0].dx; img->comps[2].dx = img->comps[0].dx; img->comps[1].dy = img->comps[0].dy; diff --git a/core/src/fxcodec/codec/fx_codec_jpx_unittest.cpp b/core/src/fxcodec/codec/fx_codec_jpx_unittest.cpp index 3add606509..2dabb5379d 100644 --- a/core/src/fxcodec/codec/fx_codec_jpx_unittest.cpp +++ b/core/src/fxcodec/codec/fx_codec_jpx_unittest.cpp @@ -464,3 +464,79 @@ TEST(fxcodec, DecodeDataSeek) { EXPECT_EQ(kReadError, opj_read_from_memory(buffer, 1, &dd)); EXPECT_EQ(0xbd, buffer[0]); } + +TEST(fxcodec, YUV420ToRGB) { + opj_image_comp_t u; + memset(&u, 0, sizeof(u)); + u.dx = 1; + u.dy = 1; + u.w = 16; + u.h = 16; + u.prec = 8; + u.bpp = 8; + u.data = FX_Alloc(OPJ_INT32, u.w * u.h); + memset(u.data, 0, u.w * u.h * sizeof(OPJ_INT32)); + opj_image_comp_t v; + memset(&v, 0, sizeof(v)); + v.dx = 1; + v.dy = 1; + v.w = 16; + v.h = 16; + v.prec = 8; + v.bpp = 8; + v.data = FX_Alloc(OPJ_INT32, v.w * v.h); + memset(v.data, 0, v.w * v.h * sizeof(OPJ_INT32)); + opj_image_comp_t y; + memset(&y, 0, sizeof(y)); + y.dx = 1; + y.dy = 1; + y.prec = 8; + y.bpp = 8; + opj_image_t img; + memset(&img, 0, sizeof(img)); + img.numcomps = 3; + img.color_space = OPJ_CLRSPC_SYCC; + img.comps = FX_Alloc(opj_image_comp_t, 3); + const struct { + int w; + bool expected; + } cases[] = {{0, false}, + {1, false}, + {30, false}, + {31, true}, + {32, true}, + {33, true}, + {34, false}, + {UINT_MAX, false}}; + for (int i = 0; i < sizeof(cases) / sizeof(cases[0]); ++i) { + y.w = cases[i].w; + y.h = y.w; + img.x1 = y.w; + img.y1 = y.h; + y.data = FX_Alloc(OPJ_INT32, y.w * y.h); + memset(y.data, 1, y.w * y.h * sizeof(OPJ_INT32)); + u.data = FX_Alloc(OPJ_INT32, u.w * u.h); + memset(u.data, 0, u.w * u.h * sizeof(OPJ_INT32)); + v.data = FX_Alloc(OPJ_INT32, v.w * v.h); + memset(v.data, 0, v.w * v.h * sizeof(OPJ_INT32)); + img.comps[0] = y; + img.comps[1] = u; + img.comps[2] = v; + sycc420_to_rgb(&img); + if (cases[i].expected) { + EXPECT_EQ(img.comps[0].w, img.comps[1].w); + EXPECT_EQ(img.comps[0].h, img.comps[1].h); + EXPECT_EQ(img.comps[0].w, img.comps[2].w); + EXPECT_EQ(img.comps[0].h, img.comps[2].h); + } else { + EXPECT_NE(img.comps[0].w, img.comps[1].w); + EXPECT_NE(img.comps[0].h, img.comps[1].h); + EXPECT_NE(img.comps[0].w, img.comps[2].w); + EXPECT_NE(img.comps[0].h, img.comps[2].h); + } + FX_Free(img.comps[0].data); + FX_Free(img.comps[1].data); + FX_Free(img.comps[2].data); + } + FX_Free(img.comps); +} -- cgit v1.2.3