Skip to content

GitLab

  • Menu
Projects Groups Snippets
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • gdk-pixbuf gdk-pixbuf
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 80
    • Issues 80
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 14
    • Merge requests 14
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Monitor
    • Monitor
    • Incidents
  • Packages & Registries
    • Packages & Registries
    • Container Registry
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • GNOME
  • gdk-pixbufgdk-pixbuf
  • Issues
  • #59

Closed
Open
Created Jan 01, 2017 by Bugzilla@bugzilla-migration💬Reporter

bmp: left shift by 24 bits does not always fit in an "int" (clrUsed)

Submitted by Tobias Mueller @tobiasmue

Link to original bug (#776695)

Description

io-bmp.c:335:28: runtime error: left shift of 222 by 24 places cannot be represented in type 'int'
io-bmp.c:337:26: runtime error: shift exponent 32 is too large for 32-bit type 'int'

(process:20881): GLib-GObject-WARNING **: value "-2147352580" of type 'gint' is invalid or out of range for property 'rowstride' of type 'gint'
fish: “./tests/pixbuf-read /tmp/comput…” terminated by signal SIGTRAP (Trace or breakpoint trap)

We can see the offending shift here:

    if (State->Header.size == 12)
        clrUsed = 1 <`< State->`Header.depth;
    else
        clrUsed = (BIH[35] << 24) + (BIH[34] << 16) + (BIH[33] << 8) + (BIH[32]);

        if (clrUsed > (1U <`< State->`Header.depth))
          {
        g_set_error_literal (error

The values get promoted to (signed) ints. If you shift by 24 digits and the high bit is set, then you overflow the range of the signed int. So the BIH[35], which gets promoted to an int, should be marked as unsigned before shifting. But also, n_colors is assigned the value of clrUsed later and n_colors seems to be an unsigned type:

struct headerpair {
    guint32 size;
    gint32 width;
    gint32 height;
    guint depth;
    guint Negative;     /* Negative = 1 -> top down BMP,
                   Negative = 0 -> bottom up BMP */
    guint  n_colors;
};

So I guess that clrUsed should also be unsigned. That would allow a shift of 1 by up to 31 digits.

We cannot shift a 1 by 32 places in a regular 32bit int, being it unsigned or not. So I think the pragmatic approach is to check for the depth being strictly less than 31. I don't know whether this breaks some images. I guess it doesn't, though.

This seems to also be CID 1388521.

Version: git master

Edited May 30, 2018 by Emmanuele Bassi
To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information
Assignee
Assign to
Time tracking