diff --git a/src_c/draw.c b/src_c/draw.c index a5667b8c94..4accefdef1 100644 --- a/src_c/draw.c +++ b/src_c/draw.c @@ -1425,8 +1425,12 @@ clip_line(SDL_Surface *surf, SDL_Rect surf_clip_rect, int *x1, int *y1, } static int -set_at(SDL_Surface *surf, SDL_Rect surf_clip_rect, int x, int y, Uint32 color) +set_at(SDL_Surface *surf, SDL_Rect surf_clip_rect, intptr_t x, intptr_t y, + Uint32 color) { + // x and y should be intptr_t so that (y * surf->pitch) + x doesn't + // overflow the int bounds in the case of very large surfaces and + // drawing on the edge of them Uint8 *pixels = (Uint8 *)surf->pixels; if (x < surf_clip_rect.x || x >= surf_clip_rect.x + surf_clip_rect.w || @@ -1434,6 +1438,37 @@ set_at(SDL_Surface *surf, SDL_Rect surf_clip_rect, int x, int y, Uint32 color) return 0; } +#if INTPTR_MAX == INT32_MAX + // Need to bounds check operation on 32-bit systems + // first check y * surf->pitch doesn't overflow INT32_MAX + // or underflow INT32_MIN + // a * b > c if and only if a > c / b || a * b < c if and only if a < c / b + if ((y > (INT32_MAX / surf->pitch)) || (y < (INT32_MIN / surf->pitch))) { + return 0; + } + + intptr_t product = y * surf->pitch; + + // now that the multiplication is fine, check that the + // addition is fine + // this is complicated, based on the sign of x + if (x >= 0) { + // product + x > INT32_MAX if and only if product > INT32_MAX - x + // product + x < INT32_MIN cannot happen since we have something that + // is no smaller than INT32_MIN added to something no smaller than 0 + if (product > (INT32_MAX - x)) { + return 0; + } + } + else { + // product + x > INT32_MAX cannot happen for a similar reason + // product + x < INT32_MIN if and only if product < INT32_MIN - x + if (product < (INT32_MIN - x)) { + return 0; + } + } +#endif + switch (PG_SURF_BytesPerPixel(surf)) { case 1: *((Uint8 *)pixels + y * surf->pitch + x) = (Uint8)color; @@ -1459,7 +1494,7 @@ static void set_and_check_rect(SDL_Surface *surf, SDL_Rect surf_clip_rect, int x, int y, Uint32 color, int *drawn_area) { - if (set_at(surf, surf_clip_rect, x, y, color)) { + if (set_at(surf, surf_clip_rect, (intptr_t)x, (intptr_t)y, color)) { add_pixel_to_drawn_list(x, y, drawn_area); } } @@ -1788,7 +1823,7 @@ drawhorzlineclip(SDL_Surface *surf, SDL_Rect surf_clip_rect, Uint32 color, } if (x1 == x2) { - set_at(surf, surf_clip_rect, x1, y1, color); + set_at(surf, surf_clip_rect, (intptr_t)x1, (intptr_t)y1, color); return; } drawhorzline(surf, color, x1, y1, x2); diff --git a/test/draw_test.py b/test/draw_test.py index cb72452fdf..1d6387af7c 100644 --- a/test/draw_test.py +++ b/test/draw_test.py @@ -1,4 +1,5 @@ import math +import os import sys import unittest import warnings @@ -1846,6 +1847,20 @@ def test_line_clipping_with_thickness(self): "start={}, end={}".format(end_points[n], start_points[n]), ) + @unittest.skipIf(pygame.system.get_total_ram() < 4096, "Surface is too large") + def test_line_draw_large_surf_regression(self): + """Regression test for https://github.com/pygame-community/pygame-ce/issues/2961""" + try: + surface = pygame.Surface((14457, 37200)) + except pygame.error: + self.skipTest( + "Surface too large to run this test in this environment, or too many background tasks" + ) + + point1 = [400, 37135] + point2 = [401, 37136] + pygame.draw.line(surface, (255, 255, 255), point1, point2, 1) + ### Lines Testing #############################################################