Harden input validation and robustness across decoder, encoder, and library

CMakeLists.txt

@@ -70,6 +70,8 @@ option(OAPV_BUILD_SHARED_LIB "Build oapv shared library" ON)
 
 option(ENABLE_COVERAGE "Enable code coverage measurement" OFF)
 
+option(OAPV_ENABLE_SANITIZERS "Enable AddressSanitizer and UndefinedBehaviorSanitizer" OFF)
+
 if(OAPV_BUILD_APPS AND OAPV_APP_STATIC_BUILD AND NOT OAPV_BUILD_STATIC_LIB)
   message(FATAL_ERROR "Cannot build static apps without static lib.")
 endif()
@@ -130,6 +132,12 @@ elseif(UNIX OR MINGW)
       set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} --coverage")
       set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} --coverage")
     endif()
+
+    if(OAPV_ENABLE_SANITIZERS)
+      # -fno-omit-frame-pointer overrides the earlier -fomit-frame-pointer
+      set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address,undefined -fno-omit-frame-pointer")
+      set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address,undefined")
+    endif()
 else()
     message("Unknown compiler")
 endif()

app/oapv_app_args.h

@@ -465,6 +465,7 @@ static int args_parse(args_parser_t *args, int argc, const char *argv[],
     for(i = 1; i < argc; i++) {
         if(!strcmp(argv[i], "--" ARGS_KEY_LONG_CONFIG)) {
             if(i + 1 < argc) {
+                if(num_configs >= ARGS_MAX_NUM_CONF_FILES) break;
                 num_configs++;
                 pos_conf_files[num_configs - 1] = i + 1;
             }

app/oapv_app_dec.c

@@ -34,8 +34,7 @@
 #include "oapv_app_args.h"
 #include "oapv_app_y4m.h"
 
-#define MAX_BS_BUF            128 * 1024 * 1024 /* byte */
-#define MAX_NUM_METADATA_PLDS 128 /* max number of metadata payloads per access unit */
+#define MAX_BS_BUF 128 * 1024 * 1024 /* byte */
 
 // check generic frame or not
 #define IS_NON_AUX_FRM(frm) (((frm)->pbu_type == OAPV_PBU_TYPE_PRIMARY_FRAME) || ((frm)->pbu_type == OAPV_PBU_TYPE_NON_PRIMARY_FRAME))
@@ -238,7 +237,7 @@ static int read_bitstream(FILE *fp, unsigned char *bs_buf, int *bs_buf_size)
             return 0;
         }
         else if(ret < 0) {
-            logerr("Cannot read bitstream size!\n");
+            logerr("ERR: Cannot read bitstream size!\n");
             return -1;
         }
         if(au_size > 0) {
@@ -250,7 +249,7 @@ static int read_bitstream(FILE *fp, unsigned char *bs_buf, int *bs_buf_size)
             while(au_size > 0) {
                 /* read byte */
                 if(1 != fread(&b, 1, 1, fp)) {
-                    logerr("Cannot read bitstream!\n");
+                    logerr("ERR: Cannot read bitstream!\n");
                     return -1;
                 }
                 bs_buf[read_size] = b;
@@ -261,12 +260,12 @@ static int read_bitstream(FILE *fp, unsigned char *bs_buf, int *bs_buf_size)
         }
         else {
             /* size field present but zero: malformed bitstream */
-            logerr("Cannot read bitstream size!\n");
+            logerr("ERR: Cannot read bitstream size!\n");
             return -1;
         }
     }
     else {
-        logerr("Cannot seek bitstream!\n");
+        logerr("ERR: Cannot seek bitstream!\n");
         return -1;
     }
     return read_size + 4;
@@ -281,7 +280,7 @@ static int set_extra_config(oapvd_t id, args_var_t *args_vars)
         size = 4;
         ret = oapvd_config(id, OAPV_CFG_SET_USE_FRM_HASH, &value, &size);
         if(OAPV_FAILED(ret)) {
-            logerr("failed to set config for using frame hash\n");
+            logerr("ERR: failed to set config for using frame hash\n");
             return -1;
         }
     }
@@ -290,7 +289,7 @@ static int set_extra_config(oapvd_t id, args_var_t *args_vars)
         size = 4;
         ret = oapvd_config(id, OAPV_CFG_SET_DISABLE_COMPANDING, &value, &size);
         if(OAPV_FAILED(ret)) {
-            logerr("failed to set config for disabling companding process\n");
+            logerr("ERR: failed to set config for disabling companding process\n");
             return -1;
         }
     }
@@ -565,6 +564,7 @@ int dec_api_set_0(args_var_t *args_var, FILE *fp_bs, int is_y4m)
 
         /* main decoding block */
         bitb.addr = bs_buf;
+        bitb.bsize = MAX_BS_BUF;
         bitb.ssize = bs_buf_size;
         memset(&stat, 0, sizeof(oapvd_stat_t));
 
@@ -599,8 +599,8 @@ int dec_api_set_0(args_var_t *args_var, FILE *fp_bs, int is_y4m)
             }
 
             /* validate number of metadata payloads */
-            if(num_plds < 0 || num_plds > MAX_NUM_METADATA_PLDS) {
-                logerr("ERR: invalid number of metadata payloads (%d), valid range is 0-%d\n", num_plds, MAX_NUM_METADATA_PLDS);
+            if(num_plds < 0 || num_plds > OAPV_MAX_NUM_META_PAYLOADS) {
+                logerr("ERR: invalid number of metadata payloads (%d), valid range is 0-%d\n", num_plds, OAPV_MAX_NUM_META_PAYLOADS);
                 goto END;
             }
 
@@ -706,6 +706,9 @@ int dec_api_set_0(args_var_t *args_var, FILE *fp_bs, int is_y4m)
     if(imgb_w != NULL)
         imgb_w->release(imgb_w);
 
+    if(bs_buf != NULL)
+        free(bs_buf);
+
     return 0;
 }
 
@@ -740,7 +743,7 @@ static int read_u32(FILE *fp, unsigned int * val)
 static int read_pbu(FILE *fp, unsigned char *pbu, unsigned int pbu_size)
 {
     if(pbu_size != fread(pbu, 1, pbu_size, fp)) {
-        logerr("Cannot read PDU!\n");
+        logerr("ERR: Cannot read PDU!\n");
         return -1;
     }
     return 0;
@@ -951,6 +954,7 @@ int dec_api_set_1(args_var_t *args_var, FILE *fp_bs, int is_y4m)
 
                 // start to decode a frame
                 bitb.addr = pbu;
+                bitb.bsize = pbu_size;
                 bitb.ssize = pbu_size;
 
                 clk_beg = oapv_clk_get();

app/oapv_app_enc.c

@@ -309,7 +309,7 @@ typedef struct args_var {
 
     char           width[16];
     char           height[16];
-    char           fps[16];
+    char           fps[24];
 
     char           qp[16];
     char           qp_offset_c1[16];
@@ -520,7 +520,7 @@ static int set_extra_config(oapve_t id, args_var_t *vars, oapve_param_t *param)
     if(vars->hash) {
         value = 1;
         size = 4;
-        ret = oapve_config(id, OAPV_CFG_SET_USE_FRM_HASH, &value, &size);
+        ret = oapve_config(id, OAPV_CFG_FRM(OAPV_CFG_SET_USE_FRM_HASH, 0), &value, &size);
         if(OAPV_FAILED(ret)) {
             logerr("ERR: failed to set config for using frame hash\n");
             return -1;
@@ -706,7 +706,7 @@ static int family_to_bitrate(char * family, oapve_param_t *param)
 #define UPDATE_A_PARAM_W_KEY_VAL(param, key, val) \
     if(strlen(val) > 0) { \
         if(OAPV_FAILED(oapve_param_parse(param, key, val))) { \
-            logerr("input value (%s) of %s is invalid\n", val, key); \
+            logerr("ERR: input value (%s) of %s is invalid\n", val, key); \
             return -1; \
         } \
     }
@@ -719,6 +719,11 @@ static int update_param(args_var_t *vars, oapve_param_t *param)
 
     UPDATE_A_PARAM_W_KEY_VAL(param, "width", vars->width);
     UPDATE_A_PARAM_W_KEY_VAL(param, "height", vars->height);
+    if(param->w <= 0 || param->w > IMGB_MAX_W || param->h <= 0 || param->h > IMGB_MAX_H) {
+        logerr("ERR: frame dimensions %dx%d out of range (max %dx%d)\n",
+               param->w, param->h, IMGB_MAX_W, IMGB_MAX_H);
+        return -1;
+    }
     UPDATE_A_PARAM_W_KEY_VAL(param, "fps", vars->fps);
 
     UPDATE_A_PARAM_W_KEY_VAL(param, "qp", vars->qp);
@@ -782,7 +787,7 @@ static int parse_master_display(const char* data_string, oapvm_payload_mdcv_t *m
     // Check if sscanf successfully assigned all expected fields (10 numerical values).
     const int expected_fields = 10;
     if (assigned_fields != expected_fields) {
-        logerr("Parsing error: master display color volume information");
+        logerr("ERR: Parsing error: master display color volume information");
         return -1;
     }
     return 0; // Success
@@ -1127,9 +1132,11 @@ int main(int argc, const char **argv)
             }
             else {
                 imgb_r = imgb_create(param->w, param->h, OAPV_CS_SET(cfmt, args_var->input_depth, 0));
+                if(imgb_r == NULL) { ret = -1; goto ERR; }
                 ifrms.frm[i].imgb = imgb_create(param->w, param->h, OAPV_CS_SET(cfmt, codec_depth, 0));
             }
         }
+        if(ifrms.frm[i].imgb == NULL) { ret = -1; goto ERR; }
 
         if(is_rec) {
             if(args_var->input_depth == codec_depth) {
@@ -1142,9 +1149,11 @@ int main(int argc, const char **argv)
                 else
                 {
                     imgb_w = imgb_create(param->w, param->h, OAPV_CS_SET(cfmt, args_var->input_depth, 0));
+                    if(imgb_w == NULL) { ret = -1; goto ERR; }
                     rfrms.frm[i].imgb = imgb_create(param->w, param->h, OAPV_CS_SET(cfmt, codec_depth, 0));
                 }
             }
+            if(rfrms.frm[i].imgb == NULL) { ret = -1; goto ERR; }
             rfrms.num_frms++;
         }
         ifrms.num_frms++;

app/oapv_app_util.h

@@ -322,7 +322,7 @@ oapv_imgb_t *imgb_create(int w, int h, int cs)
 
     /* reject invalid or out-of-range resolution */
     if(w <= 0 || h <= 0 || w > IMGB_MAX_W || h > IMGB_MAX_H || bd <= 0) {
-        logerr("invalid image parameter (w=%d, h=%d, byte-depth=%d)\n", w, h, bd);
+        logerr("ERR: invalid image parameter (w=%d, h=%d, byte-depth=%d)\n", w, h, bd);
         goto ERR;
     }
 
@@ -385,7 +385,7 @@ oapv_imgb_t *imgb_create(int w, int h, int cs)
     return imgb;
 
 ERR:
-    logerr("cannot create image buffer\n");
+    logerr("ERR: cannot create image buffer\n");
     if(imgb) {
         for(int i = 0; i < OAPV_MAX_CC; i++) {
             if(imgb->a[i])
@@ -407,11 +407,11 @@ static int imgb_read(FILE *fp, oapv_imgb_t *img, int width, int height, int is_y
         if(6 != fread(t_buf, 1, 6, fp))
             return -1;
         if(memcmp(t_buf, "FRAME", 5)) {
-            logerr("Loss of framing in Y4M input data\n");
+            logerr("ERR: Loss of framing in Y4M input data\n");
             return -1;
         }
         if(t_buf[5] != '\n') {
-            logerr("Error parsing Y4M frame header\n");
+            logerr("ERR: parsing Y4M frame header\n");
             return -1;
         }
     }
@@ -431,7 +431,7 @@ static int imgb_read(FILE *fp, oapv_imgb_t *img, int width, int height, int is_y
         f_h = height;
     }
     else {
-        logerr("unsupported bit-depth (%d)\n", bit_depth);
+        logerr("ERR: unsupported bit-depth (%d)\n", bit_depth);
         return -1;
     }
 
@@ -493,14 +493,20 @@ static int imgb_write(char *fname, oapv_imgb_t *imgb)
 {
     unsigned char *p8;
     int            i, j, bd;
+    int            chroma_format, bit_depth;
     FILE          *fp;
 
-    int            chroma_format = OAPV_CS_GET_FORMAT(imgb->cs);
-    int            bit_depth = OAPV_CS_GET_BIT_DEPTH(imgb->cs);
+    if(imgb == NULL) {
+        logerr("ERR: null image buffer in image write\n");
+        return -1;
+    }
+
+    chroma_format = OAPV_CS_GET_FORMAT(imgb->cs);
+    bit_depth = OAPV_CS_GET_BIT_DEPTH(imgb->cs);
 
     fp = fopen(fname, "ab");
     if(fp == NULL) {
-        logerr("cannot open file = %s\n", fname);
+        logerr("ERR: cannot open file = %s\n", fname);
         return -1;
     }
     if(bit_depth == 8 && (chroma_format == OAPV_CF_YCBCR400 || chroma_format == OAPV_CF_YCBCR420 || chroma_format == OAPV_CF_YCBCR422 ||
@@ -514,7 +520,7 @@ static int imgb_write(char *fname, oapv_imgb_t *imgb)
         bd = 2;
     }
     else {
-        logerr("cannot support the color space\n");
+        logerr("ERR: cannot support the color space\n");
         fclose(fp);
         return -1;
     }
@@ -532,6 +538,35 @@ static int imgb_write(char *fname, oapv_imgb_t *imgb)
     return 0;
 }
 
+// checks src/dst are compatible and both buffers hold the copy extent
+static int imgb_cpy_is_valid(oapv_imgb_t *dst, oapv_imgb_t *src)
+{
+    if(src == NULL || dst == NULL) {
+        logerr("ERR: null image buffer in image copy\n");
+        return 0;
+    }
+    if(src->np < 1 || src->np > OAPV_MAX_CC || dst->np != src->np) {
+        logerr("ERR: mismatched plane count in image copy\n");
+        return 0;
+    }
+    int src_bytes = OAPV_CS_GET_BYTE_DEPTH(src->cs);
+    int dst_bytes = OAPV_CS_GET_BYTE_DEPTH(dst->cs);
+    for(int i = 0; i < src->np; i++) {
+        int w = src->w[i] > src->aw[i] ? src->w[i] : src->aw[i];
+        int h = src->h[i] > src->ah[i] ? src->h[i] : src->ah[i];
+        if(src->a[i] == NULL || dst->a[i] == NULL || w < 0 || h < 0) {
+            logerr("ERR: invalid plane in image copy\n");
+            return 0;
+        }
+        if((long long)(h - 1) * src->s[i] + (long long)w * src_bytes > src->bsize[i] ||
+           (long long)(h - 1) * dst->s[i] + (long long)w * dst_bytes > dst->bsize[i]) {
+            logerr("ERR: buffer too small in image copy\n");
+            return 0;
+        }
+    }
+    return 1;
+}
+
 static void imgb_cpy_plane(oapv_imgb_t *dst, oapv_imgb_t *src)
 {
     int            i, j;
@@ -654,9 +689,17 @@ static void imgb_cpy_shift_right(oapv_imgb_t *dst, oapv_imgb_t *src, int shift)
 static void imgb_cpy(oapv_imgb_t *dst, oapv_imgb_t *src)
 {
     int i, bd_src, bd_dst;
+
+    if(!imgb_cpy_is_valid(dst, src)) return;
+
     bd_src = OAPV_CS_GET_BIT_DEPTH(src->cs);
     bd_dst = OAPV_CS_GET_BIT_DEPTH(dst->cs);
 
+    if(bd_src < 8 || bd_src > 16 || bd_dst < 8 || bd_dst > 16) {
+        logerr("ERR: unsupported bit depth in image copy\n");
+        return;
+    }
+
     if(src->cs == dst->cs) {
         imgb_cpy_plane(dst, src);
     }
@@ -708,7 +751,7 @@ static void measure_psnr(oapv_imgb_t *org, oapv_imgb_t *rec, double psnr[4], int
 
             for(j = 0; j < org->h[i]; j++) {
                 for(k = 0; k < org->w[i]; k++) {
-                    sum[i] += (o[k] - r[k]) * (o[k] - r[k]);
+                    sum[i] += (double)(o[k] - r[k]) * (o[k] - r[k]);
                 }
 
                 o += org->s[i];
@@ -734,7 +777,7 @@ static void measure_psnr(oapv_imgb_t *org, oapv_imgb_t *rec, double psnr[4], int
                         sum[i] += (((int)o[k] - (int)r[k]) >> 6) * (((int)o[k] - (int)r[k]) >> 6);
                     }
                     else {
-                        sum[i] += (o[k] - r[k]) * (o[k] - r[k]);
+                        sum[i] += (double)(o[k] - r[k]) * (o[k] - r[k]);
                     }
                 }
                 o = (unsigned short *)((unsigned char *)o + org->s[i]);
@@ -752,7 +795,7 @@ static int write_data(char *fname, unsigned char *data, int size)
 
     fp = fopen(fname, "ab");
     if(fp == NULL) {
-        logerr("cannot open the output file=%s\n", fname);
+        logerr("ERR: cannot open the output file=%s\n", fname);
         return -1;
     }
     fwrite(data, 1, size, fp);
@@ -765,7 +808,7 @@ static int clear_data(char *fname)
     FILE *fp;
     fp = fopen(fname, "wb");
     if(fp == NULL) {
-        logerr("cannot remove file (%s)\n", fname);
+        logerr("ERR: cannot remove file (%s)\n", fname);
         return -1;
     }
     fclose(fp);