Menu ▾ ▴

#147 Bugfix for #339

None
pending
Hervé Drolon
None
5
2021-11-01
2021-09-12
Patrick Pelletier
No

Makes sure that memcpy in FreeImage_FlipVertical is called with size_t and doesn't truncate or wraparound.

Fixes loading of very large exr files.

Addresses bug #339 (https://sourceforge.net/p/freeimage/bugs/339/)

Thanks!

@@ -139,2 +139,2 @@ FreeImage_FlipVertical(FIBITMAP *src) {
-   unsigned pitch  = FreeImage_GetPitch(src);
-   unsigned height = FreeImage_GetHeight(src);
+   size_t pitch  = FreeImage_GetPitch(src);
+   size_t height = FreeImage_GetHeight(src);
@@ -148,2 +148,2 @@ FreeImage_FlipVertical(FIBITMAP *src) {
-   unsigned line_s = 0;
-   unsigned line_t = (height-1) * pitch;
+   size_t line_s = 0;
+   size_t line_t = (height-1) * pitch;
@@ -151 +151 @@ FreeImage_FlipVertical(FIBITMAP *src) {
-   for(unsigned y = 0; y < height/2; y++) {
+   for(size_t y = 0; y < height/2; y++) {
2 Attachments
Flip.cpp
Flip.cpp.diff

Discussion

  • Mihail Naydenov

    Mihail Naydenov - 2021-09-19

    This is very interesting. Where exactly does it overflow. You changed many unsigned to size_t, but this way we mask the issue. Can you please add a comment where the size_t is required to draw attention, please!

     

  • Anonymous

    Anonymous - 2021-09-20

    Hi Mihail!
    line_s and line_t must be size_t to prevent wraparound into negative values. pitch and height could remain as int(though why I can't imagine).

     

  • Patrick Pelletier

    Patrick Pelletier - 2021-09-22

    Hi Mihail!

            memcpy(Mid, From + line_s, pitch);
        memcpy(From + line_s, From + line_t, pitch);
        memcpy(From + line_t, Mid, pitch);

    For example if the sum of From and line_s is greater than INT_MAX, memcopy receives a negative offset. Which explains the result outlined in the bug report.

    Only line_s and line_t really need to be of type size_t.

    (the above anonymous post is mine)

     

    Last edit: Patrick Pelletier 2021-09-22

  • Patrick Pelletier

    Patrick Pelletier - 2021-09-24

    Actually, (height-1) * pitchmight also overflow, so they should also be size_t.

     

  • Hervé Drolon

    Hervé Drolon - 2021-11-01
    • status: open --> pending
    • assigned_to: Hervé Drolon
    • Group: -->
     

  • Hervé Drolon

    Hervé Drolon - 2021-11-01

    Hi,

    Thanks for the fix, it is now fixed in the SVN.

    Hervé

     

Anonymous
Anonymous

Add attachments
Cancel