Browse Source

Prevent potential overflow in rectangle functions

We're limiting the functions to rects with positions and sizes < 1 billion for speed, which is totally fine for most SDL use cases. If you need rectangles larger than that, you can roll your own functions that use 64-bit intermediate values and do proper overflow handling of output values.

Fixes https://github.com/libsdl-org/SDL/issues/8879
Sam Lantinga 9 months ago
parent
commit
bd27b89903
3 changed files with 43 additions and 0 deletions
  1. 2 0
      src/video/SDL_rect.c
  2. 29 0
      src/video/SDL_rect_impl.h
  3. 12 0
      test/testautomation_rect.c

+ 2 - 0
src/video/SDL_rect.c

@@ -91,6 +91,7 @@ SDL_bool SDL_GetSpanEnclosingRect(int width, int height,
 #define BIGSCALARTYPE            Sint64
 #define COMPUTEOUTCODE           ComputeOutCode
 #define ENCLOSEPOINTS_EPSILON    1
+#define SDL_RECT_CAN_OVERFLOW    SDL_RectCanOverflow
 #define SDL_HASINTERSECTION      SDL_HasRectIntersection
 #define SDL_INTERSECTRECT        SDL_GetRectIntersection
 #define SDL_RECTEMPTY            SDL_RectEmpty
@@ -105,6 +106,7 @@ SDL_bool SDL_GetSpanEnclosingRect(int width, int height,
 #define BIGSCALARTYPE            double
 #define COMPUTEOUTCODE           ComputeOutCodeFloat
 #define ENCLOSEPOINTS_EPSILON    0.0f
+#define SDL_RECT_CAN_OVERFLOW    SDL_RectCanOverflowFloat
 #define SDL_HASINTERSECTION      SDL_HasRectIntersectionFloat
 #define SDL_INTERSECTRECT        SDL_GetRectIntersectionFloat
 #define SDL_RECTEMPTY            SDL_RectEmptyFloat

+ 29 - 0
src/video/SDL_rect_impl.h

@@ -21,6 +21,19 @@
 
 /* This file is #included twice to support int and float versions with the same code. */
 
+static SDL_bool SDL_RECT_CAN_OVERFLOW(const RECTTYPE *rect)
+{
+    if (rect->x <= (SCALARTYPE)(SDL_MIN_SINT32 / 2) ||
+        rect->x >= (SCALARTYPE)(SDL_MAX_SINT32 / 2) ||
+        rect->y <= (SCALARTYPE)(SDL_MIN_SINT32 / 2) ||
+        rect->y >= (SCALARTYPE)(SDL_MAX_SINT32 / 2) ||
+        rect->w >= (SCALARTYPE)(SDL_MAX_SINT32 / 2) ||
+        rect->h >= (SCALARTYPE)(SDL_MAX_SINT32 / 2)) {
+        return SDL_TRUE;
+    }
+    return SDL_FALSE;
+}
+
 SDL_bool SDL_HASINTERSECTION(const RECTTYPE *A, const RECTTYPE *B)
 {
     SCALARTYPE Amin, Amax, Bmin, Bmax;
@@ -31,6 +44,10 @@ SDL_bool SDL_HASINTERSECTION(const RECTTYPE *A, const RECTTYPE *B)
     } else if (!B) {
         SDL_InvalidParamError("B");
         return SDL_FALSE;
+    } else if (SDL_RECT_CAN_OVERFLOW(A) ||
+               SDL_RECT_CAN_OVERFLOW(B)) {
+        SDL_SetError("Potential rect math overflow");
+        return SDL_FALSE;
     } else if (SDL_RECTEMPTY(A) || SDL_RECTEMPTY(B)) {
         return SDL_FALSE; /* Special cases for empty rects */
     }
@@ -76,6 +93,10 @@ SDL_bool SDL_INTERSECTRECT(const RECTTYPE *A, const RECTTYPE *B, RECTTYPE *resul
     } else if (!B) {
         SDL_InvalidParamError("B");
         return SDL_FALSE;
+    } else if (SDL_RECT_CAN_OVERFLOW(A) ||
+               SDL_RECT_CAN_OVERFLOW(B)) {
+        SDL_SetError("Potential rect math overflow");
+        return SDL_FALSE;
     } else if (!result) {
         SDL_InvalidParamError("result");
         return SDL_FALSE;
@@ -124,6 +145,10 @@ int SDL_UNIONRECT(const RECTTYPE *A, const RECTTYPE *B, RECTTYPE *result)
         return SDL_InvalidParamError("A");
     } else if (!B) {
         return SDL_InvalidParamError("B");
+    } else if (SDL_RECT_CAN_OVERFLOW(A) ||
+               SDL_RECT_CAN_OVERFLOW(B)) {
+        SDL_SetError("Potential rect math overflow");
+        return SDL_FALSE;
     } else if (!result) {
         return SDL_InvalidParamError("result");
     } else if (SDL_RECTEMPTY(A)) { /* Special cases for empty Rects */
@@ -300,6 +325,9 @@ SDL_bool SDL_INTERSECTRECTANDLINE(const RECTTYPE *rect, SCALARTYPE *X1, SCALARTY
     if (!rect) {
         SDL_InvalidParamError("rect");
         return SDL_FALSE;
+    } else if (SDL_RECT_CAN_OVERFLOW(rect)) {
+        SDL_SetError("Potential rect math overflow");
+        return SDL_FALSE;
     } else if (!X1) {
         SDL_InvalidParamError("X1");
         return SDL_FALSE;
@@ -430,6 +458,7 @@ SDL_bool SDL_INTERSECTRECTANDLINE(const RECTTYPE *rect, SCALARTYPE *X1, SCALARTY
 #undef BIGSCALARTYPE
 #undef COMPUTEOUTCODE
 #undef ENCLOSEPOINTS_EPSILON
+#undef SDL_RECT_CAN_OVERFLOW
 #undef SDL_HASINTERSECTION
 #undef SDL_INTERSECTRECT
 #undef SDL_RECTEMPTY

+ 12 - 0
test/testautomation_rect.c

@@ -2,6 +2,7 @@
  * Original code: automated SDL rect test written by Edgar Simo "bobbens"
  * New/updated tests: aschiffler at ferzkopp dot net
  */
+#include <limits.h>
 #include <SDL3/SDL.h>
 #include <SDL3/SDL_test.h>
 #include "testautomation_suites.h"
@@ -161,6 +162,17 @@ static int rect_testIntersectRectAndLine(void *arg)
     intersected = SDL_GetRectAndLineIntersection(&rect, &x1, &y1, &x2, &y2);
     validateIntersectRectAndLineResults(intersected, SDL_TRUE, &rect, &refRect, x1, y1, x2, y2, 31, 0, 0, 31);
 
+    /* Test some overflow cases */
+    refRect.x = INT_MAX - 4;
+    refRect.y = INT_MAX - 4;
+    x1 = INT_MAX;
+    y1 = INT_MIN;
+    x2 = INT_MIN;
+    y2 = INT_MAX;
+    rect = refRect;
+    intersected = SDL_GetRectAndLineIntersection(&rect, &x1, &y1, &x2, &y2);
+    validateIntersectRectAndLineResults(intersected, SDL_FALSE, &rect, &refRect, x1, y1, x2, y2, x1, y1, x2, y2);
+
     return TEST_COMPLETED;
 }