From 9b889c3fabecef2d39bb87f740b7593acd700763 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Arttu=20Yl=C3=A4-Outinen?= <arttu.yla-outinen@tut.fi>
Date: Sun, 29 Jan 2017 14:22:26 +0900
Subject: [PATCH] Fix reading ROI files

- Checks the return value of fopen when opening the ROI file. Fixes
  a segfault when the file cannot be opened.
- Check that the width and height are positive. Fixes reading past the
  end of the delta QP array in kvz_set_lcu_lambda_and_qp.
- Check for overflow in width * height. Fixes an overflow resulting in
  a segfault.
- Properly check that fscanf succeeds. Fixes silently accepting ROI
  files that are too short.
- Properly close the FILE pointer.
---
 src/cfg.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/src/cfg.c b/src/cfg.c
index 2f996b6e..91f8e25a 100644
--- a/src/cfg.c
+++ b/src/cfg.c
@@ -986,27 +986,53 @@ int kvz_config_parse(kvz_config *cfg, const char *name, const char *value)
     // First number is width, second number is height,
     // then follows width * height number of dqp values.
     FILE* f = fopen(value, "rb");
+    if (!f) {
+      fprintf(stderr, "Could not open ROI file.\n");
+      return 0;
+    }
+
     int width = 0;
     int height = 0;
     if (!fscanf(f, "%d", &width) || !fscanf(f, "%d", &height)) {
       fprintf(stderr, "Failed to read ROI size.\n");
+      fclose(f);
       return 0;
     }
-    cfg->roi.width = width;
+
+    if (width <= 0 || height <= 0) {
+      fprintf(stderr, "Invalid ROI size: %dx%d.\n", width, height);
+      fclose(f);
+      return 0;
+    }
+
+    const long long unsigned size = (long long unsigned) width *
+                                    (long long unsigned) height;
+    if (size > SIZE_MAX) {
+      fprintf(stderr, "Too large ROI size: %llu (maximum %zu).\n", size, SIZE_MAX);
+      return 0;
+    }
+
+    cfg->roi.width  = width;
     cfg->roi.height = height;
-    cfg->roi.dqps = malloc(width * height * sizeof(*cfg->roi.dqps));
+    // FIXME: this array is never freed
+    cfg->roi.dqps   = calloc((size_t)size, sizeof(*cfg->roi.dqps));
     if (!cfg->roi.dqps) {
       fprintf(stderr, "Failed to allocate memory for ROI table.\n");
+      fclose(f);
       return 0;
     }
-    for (int i = 0; i < width * height; ++i) {
+
+    for (int i = 0; i < size; ++i) {
       int number; // Need a pointer to int for fscanf
-      if (!fscanf(f, "%d", &number)) {
+      if (fscanf(f, "%d", &number) != 1) {
         fprintf(stderr, "Reading ROI file failed.\n");
+        fclose(f);
         return 0;
       }
       cfg->roi.dqps[i] = (uint8_t)number;
     }
+
+    fclose(f);
   }
   else
     return 0;