[Git][xorg-team/wayland/xwayland][upstream-unstable] 12 commits: dix/colormap: fix out-of-bounds read in FindColorInRootCmap
"Emilio Pozuelo Monfort (@pochu)" <[email protected]>
| Newsgroups | gmane.linux.debian.devel.x |
|---|---|
| Message-ID | <[email protected]> |
Emilio Pozuelo Monfort pushed to branch upstream-unstable at X Strike Force / wayland / xwayland Commits: d1ae432e by Peter Hutterer at 2026-07-02T17:25:15+02:00 dix/colormap: fix out-of-bounds read in FindColorInRootCmap The for loop here always iterates size times but the client controls the starting offset. When the starting pixel is non-zero (e.g., pixel=10 in a size=256 colormap), the loop reads from pentFirst[10] through pentFirst[265], reading 10 entries past the end of the array. Fix this by wrapping around once we reach size, same as FindColor() already does. Assisted-by: Claude:claude-claude-opus-4-6 (cherry picked from commit b4f2807a40fb0b149f475f3e459a3a38fd114096) Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2248> - - - - - 5c356ecf by Mikhail Dmitrichenko at 2026-07-02T17:25:15+02:00 xkb: preserve buffer on realloc failure _Concat() stored the realloc() result directly in its input pointer, so an allocation failure could drop the only reference to the original buffer when callers assigned the return value back to their destination pointer. Keep the old pointer until realloc() succeeds, avoiding the leak reported by static analysis. Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Mikhail Dmitrichenko <[email protected]> (cherry picked from commit d6c462f59927b3702a54e0e8ea2a5de7639294e6) Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2248> - - - - - 6d5761d2 by Peter Hutterer at 2026-07-02T17:25:15+02:00 glx: fix duplicate tagInfo->vendor = NULL assignment Assisted-by: Claude:claude-opus-4-6 (cherry picked from commit 66c4a2a6af70112295a5e163fe75a78d29bd3137) Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2248> - - - - - 45bb6a7b by Peter Hutterer at 2026-07-02T17:25:15+02:00 glamor: fix an error path cleanup The GL_OUT_OF_MEMORY error path returns NULL without freeing the allocated `bits` buffer or deleting the GL texture. Assisted-by: Claude:claude-opus-4-6 (cherry picked from commit 174aac67c99cb7ac26acb6613831c6db0ebd549f) Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2248> - - - - - e6695888 by Olivier Fourdan at 2026-07-02T17:25:15+02:00 dix: Silence a compiler warning in doListFontsAndAliases() Compiler complains that "resolvedlen" might be uninitialized: | dix/dixfonts.c:559:5: var_decl: Declaring variable "resolvedlen" without initializer. | dix/dixfonts.c:674:17: uninit_use: Using uninitialized value "resolvedlen". | 672| * is complete. | 673| */ | 674|-> if (resolvedlen > XLFDMAXFONTNAMELEN) { | 675| err = BadFontName; | 676| goto ContBadFontName; Most likely a false positive, while immediately after the (newly added) test, there was a memcpy() using "resolvedlen" and the compiler did not choke on that before. Either way, initializing "resolvedlen" to 0 is a small price to pay to silence the compiler warning and keep us on the safe side. Signed-off-by: Olivier Fourdan <[email protected]> (cherry picked from commit e710e570b1709d100072a8ab7d05c2aefaf41a1b) Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2248> - - - - - 64fc1525 by Olivier Fourdan at 2026-07-02T17:25:15+02:00 dix: Silent static analyzer warning | dix/dixfonts.c:849:5: var_decl: Declaring variable "namelen" without initializer. | dix/dixfonts.c:932:17: uninit_use: Using uninitialized value "namelen". | 930| c->savedNumFonts = numFonts; | 931| free(c->savedName); | 932|-> c->savedName = XNFalloc(namelen + 1); | 933| memcpy(c->savedName (cherry picked from commit f959f1e51f369ac26b6ba5953a3b022407e85b11) Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2248> - - - - - d04eb640 by Olivier Fourdan at 2026-07-02T17:25:15+02:00 dix: Silence a compiler warning in doListFontsWithInfo() Static analyzer reports: | dix/dixfonts.c:936:17: uninit_use_in_call: Using uninitialized value "name" when calling "memcpy". | 934| free(c->savedName); | 935| c->savedName = XNFalloc(namelen + 1); | 936|-> memcpy(c->savedName, name, namelen + 1); | 937| aliascount = 20; | 938| } To silence the warning, we cannot just set "name" to NULL, as this could potentially cause a NULL pointer in memcpy(), we also need to check if the value was set. If not, just bail out with a BadFontNAme error. Signed-off-by: Olivier Fourdan <[email protected]> (cherry picked from commit 5d011bf3da81595bef25ca89458249c1037a4f51) Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2248> - - - - - 5693a0a7 by Olivier Fourdan at 2026-07-02T17:25:15+02:00 Xi: Check window attribute is valid in XIChangeCursor The XInput2 request XIChangeCursor() does not specify whether the window attribute can be NULL, nor does the code in ProcXIChangeCursor(). As a result, if the window is NULL, the xserver ends up dereferencing a NULL pointer and segfaults. To avoid the issue, check the actual value passed for the window and return a BadWindow error if the window was not specified. Please note that particular error is however not documented in the XI2 protocol definition [1], yet that seems to be the only sane course of action, reading the definition of XIChangeCursor(), a valid window is to be expected. [1] https://xorg.freedesktop.org/archive/current/doc/inputproto/XI2proto.txt (cherry picked from commit 7aac5d4dc6bc934b7cae40afdfd98261a8e80427) Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2248> - - - - - 9ca6ab9d by Peter Hutterer at 2026-07-08T10:47:15+10:00 glx: free old context tag before allocating new one in CommonMakeCurrent oldTag in CommonMakeCurrent() is a pointer to cl->contextTags[...]. CommonMakeCurrent() may realloc(cl->contextTags) and thus move the memory, leaving oldTag as dangling pointer. If we then GlxFreeContextTag(oldTag) we end up writing into freed memory. Fix this by freeing oldTag before CommonMakeNewCurrent(). This vulnerability was discovered by: Anonymous working with Trend Micro Zero Day Initiative CVE-2026-56000/ZDI-CAN-30561 Fixes: 4781f2a5a8c2 ("GLX: Free the tag of the old context later") Assisted-by: Claude:claude-opus-4-6 (cherry picked from commit 2779affbdb4354e894f490e56f962527d6125043) Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2251> - - - - - 822fac44 by Peter Hutterer at 2026-07-08T10:47:28+10:00 fb/mi/glamor: reject glyphs with negative dimensions GLYPHWIDTHPIXELS and GLYPHHEIGHTPIXELS compute glyph dimensions from signed INT16 fields. A crafted PCF font can produce negative results (e.g. rightSideBearing < leftSideBearing). All callees have a while (height--) loop which will end up in OOB writes for negative height values. In the case of a negative height, some callees cast to size_t or end up with negative strides. The same pattern exists in fbPoloyGlyphBit, miPolyGlyphBlt and glamor_poly_glyph_blt_gl, so let's fix all of them in one go. Assisted-by: Claude:claude-opus-4-6 (cherry picked from commit e31efd3e106e53bfc29d499ff0a34b0d20013f2d) Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2251> - - - - - 28bf4097 by Peter Hutterer at 2026-07-08T10:47:28+10:00 glamor: reject fonts with per-glyph metrics exceeding maxbounds glamor_font_get() computes the atlas slot size from the font's declared maxbounds, but copies each glyph's bitmap using the per-glyph metrics (GLYPHHEIGHTPIXELS/GLYPHWIDTHBYTES macros). When a malicious PCF font has per-glyph metrics exceeding maxbounds, the memcpy writes past the heap-allocated atlas buffer. Negative per-glyph metrics are even worse: the loop counter wraps to ~4 billion iterations (via unsigned cast) or memcpy's size parameter wraps to SIZE_MAX. The PCF parser in libXfont2 does not recompute maxbounds from per-glyph data (only the BDF parser does), so the file's declared maxbounds values are trusted as-is. Reject fonts where any per-glyph metric is negative or exceeds the atlas slot size, falling back to software rendering which uses per-glyph metrics directly without an atlas. This vulnerability was discovered by: Anonymous working with Trend Micro Zero Day Initiative CVE-2026-55999/ZDI-CAN-30498 Assisted-by: Claude:claude-opus-4-6 (cherry picked from commit fbf7bac22e2c6bd627fb042742a23318263edae1) Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2251> - - - - - c5a47fda by Peter Hutterer at 2026-07-08T11:17:38+10:00 Bump version to 24.1.13 Signed-off-by: Peter Hutterer <[email protected]> Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2253> - - - - - 11 changed files: - Xi/xichangecursor.c - dix/colormap.c - dix/dixfonts.c - fb/fbglyph.c - glamor/glamor_font.c - glamor/glamor_glyphblt.c - glx/vndcmds.c - glx/vndservermapping.c - meson.build - mi/miglblt.c - xkb/maprules.c Changes: ===================================== Xi/xichangecursor.c ===================================== @@ -89,6 +89,9 @@ ProcXIChangeCursor(ClientPtr client) return rc; } + if (pWin == NullWindow) + return BadWindow; + if (stuff->cursor == None) { if (pWin == pWin->drawable.pScreen->root) pCursor = rootCursor; ===================================== dix/colormap.c ===================================== @@ -1288,7 +1288,7 @@ FindColorInRootCmap(ColormapPtr pmap, EntryPtr pentFirst, int size, if ((pixel = *pPixel) >= size) pixel = 0; - for (pent = pentFirst + pixel, count = size; --count >= 0; pent++, pixel++) { + for (pent = pentFirst + pixel, count = size; --count >= 0;) { if (pent->refcnt > 0 && (*comp) (pent, prgb)) { switch (channel) { case REDMAP: @@ -1305,6 +1305,13 @@ FindColorInRootCmap(ColormapPtr pmap, EntryPtr pentFirst, int size, } *pPixel = pixel; } + pixel++; + if (pixel >= size) { + pent = pentFirst; + pixel = 0; + } + else + pent++; } } ===================================== dix/dixfonts.c ===================================== @@ -556,7 +556,7 @@ doListFontsAndAliases(ClientPtr client, LFclosurePtr c) int err = Successful; FontNamesPtr names = NULL; char *name, *resolved = NULL; - int namelen, resolvedlen; + int namelen, resolvedlen = 0; int nnames; int stringLens; int i; @@ -848,8 +848,8 @@ doListFontsWithInfo(ClientPtr client, LFWIclosurePtr c) { FontPathElementPtr fpe; int err = Successful; - char *name; - int namelen; + char *name = NULL; + int namelen = 0; int numFonts; FontInfoRec fontInfo, *pFontInfo; xListFontsWithInfoReply *reply; @@ -912,6 +912,10 @@ doListFontsWithInfo(ClientPtr client, LFWIclosurePtr c) * is BadFontName, indicating the alias resolution * is complete. */ + if (!name) { + err = BadFontName; + goto ContBadFontName; + } if (c->haveSaved) { char *tmpname; int tmpnamelen; ===================================== fb/fbglyph.c ===================================== @@ -95,7 +95,7 @@ fbPolyGlyphBlt(DrawablePtr pDrawable, pglyph = FONTGLYPHBITS(pglyphBase, pci); gWidth = GLYPHWIDTHPIXELS(pci); gHeight = GLYPHHEIGHTPIXELS(pci); - if (gWidth && gHeight) { + if (gWidth > 0 && gHeight > 0) { gx = x + pci->metrics.leftSideBearing; gy = y - pci->metrics.ascent; if (glyph && gWidth <= sizeof(FbStip) * 8 && @@ -197,7 +197,7 @@ fbImageGlyphBlt(DrawablePtr pDrawable, pglyph = FONTGLYPHBITS(pglyphBase, pci); gWidth = GLYPHWIDTHPIXELS(pci); gHeight = GLYPHHEIGHTPIXELS(pci); - if (gWidth && gHeight) { + if (gWidth > 0 && gHeight > 0) { gx = x + pci->metrics.leftSideBearing; gy = y - pci->metrics.ascent; if (glyph && gWidth <= sizeof(FbStip) * 8 && ===================================== glamor/glamor_font.c ===================================== @@ -71,6 +71,9 @@ glamor_font_get(ScreenPtr screen, FontPtr font) glyph_width_pixels = font->info.maxbounds.rightSideBearing - font->info.minbounds.leftSideBearing; glyph_height = font->info.maxbounds.ascent + font->info.maxbounds.descent; + if (glyph_width_pixels <= 0 || glyph_height <= 0) + return NULL; + glyph_width_bytes = (glyph_width_pixels + 7) >> 3; glamor_font->glyph_width_pixels = glyph_width_pixels; @@ -130,7 +133,28 @@ glamor_font_get(ScreenPtr screen, FontPtr font) if (count) { char *dst; char *src = glyph->bits; - unsigned y; + int gw = GLYPHWIDTHBYTES(glyph); + int gh = GLYPHHEIGHTPIXELS(glyph); + + /* Reject fonts where any per-glyph metric is negative + * or exceeds the atlas slot size derived from maxbounds. + * The PCF parser in libXfont2 does not recompute + * maxbounds from per-glyph data, so a crafted PCF file + * can violate the maxbounds invariant. + * + * gw is passed as size_t to memcpy and a negative value + * would thus result in OOB access. + * + * Returning NULL makes glamor fall back to software + * rendering. + */ + if (gw < 0 || gh < 0 || + gw > glyph_width_bytes || gh > glyph_height) { + glDeleteTextures(1, &glamor_font->texture_id); + glamor_font->texture_id = 0; + free(bits); + return NULL; + } dst = bits; /* get offset of start of first row */ @@ -139,8 +163,8 @@ glamor_font_get(ScreenPtr screen, FontPtr font) dst += (row & 1) ? glamor_font->row_width : 0; dst += col * glyph_width_bytes; - for (y = 0; y < GLYPHHEIGHTPIXELS(glyph); y++) { - memcpy(dst, src, GLYPHWIDTHBYTES(glyph)); + for (int y = 0; y < gh; y++) { + memcpy(dst, src, gw); dst += overall_width; src += GLYPHWIDTHBYTESPADDED(glyph); } @@ -154,11 +178,14 @@ glamor_font_get(ScreenPtr screen, FontPtr font) glTexImage2D(GL_TEXTURE_2D, 0, GL_R8UI, overall_width, overall_height, 0, GL_RED_INTEGER, GL_UNSIGNED_BYTE, bits); glamor_priv->suppress_gl_out_of_memory_logging = false; - if (glGetError() == GL_OUT_OF_MEMORY) - return NULL; - free(bits); + if (glGetError() == GL_OUT_OF_MEMORY) { + glDeleteTextures(1, &glamor_font->texture_id); + glamor_font->texture_id = 0; + return NULL; + } + glamor_font->realized = TRUE; return glamor_font; ===================================== glamor/glamor_glyphblt.c ===================================== @@ -90,7 +90,7 @@ glamor_poly_glyph_blt_gl(DrawablePtr drawable, GCPtr gc, int h = GLYPHHEIGHTPIXELS(charinfo); uint8_t *glyphbits = FONTGLYPHBITS(NULL, charinfo); - if (w && h) { + if (w > 0 && h > 0) { int glyph_x = x + charinfo->metrics.leftSideBearing; int glyph_y = y - charinfo->metrics.ascent; int glyph_stride = GLYPHWIDTHBYTESPADDED(charinfo); ===================================== glx/vndcmds.c ===================================== @@ -256,6 +256,11 @@ static int CommonMakeCurrent(ClientPtr client, if (ret != Success) { return ret; } + // Free the old tag before calling CommonMakeNewCurrent(), + // which may call GlxAllocContextTag() and realloc the + // contextTags array, invalidating the oldTag pointer. + GlxFreeContextTag(oldTag); + oldTag = NULL; } if (newVendor != NULL) { @@ -266,9 +271,6 @@ static int CommonMakeCurrent(ClientPtr client, } else { reply.contextTag = 0; } - - GlxFreeContextTag(oldTag); - oldTag = NULL; } reply.contextTag = GlxCheckSwap(client, reply.contextTag); ===================================== glx/vndservermapping.c ===================================== @@ -158,7 +158,7 @@ GlxContextTagInfo *GlxLookupContextTag(ClientPtr client, GLXContextTag tag) void GlxFreeContextTag(GlxContextTagInfo *tagInfo) { if (tagInfo != NULL) { - tagInfo->vendor = NULL; + tagInfo->client = NULL; tagInfo->vendor = NULL; tagInfo->data = NULL; tagInfo->context = None; ===================================== meson.build ===================================== @@ -3,10 +3,10 @@ project('xwayland', 'c', 'buildtype=debugoptimized', 'c_std=gnu99', ], - version: '24.1.12', + version: '24.1.13', meson_version: '>= 0.56.0', ) -release_date = '2026-06-02' +release_date = '2026-07-08' add_project_arguments('-DHAVE_DIX_CONFIG_H', language: ['c', 'objc']) cc = meson.get_compiler('c') ===================================== mi/miglblt.c ===================================== @@ -143,7 +143,7 @@ miPolyGlyphBlt(DrawablePtr pDrawable, GC * pGC, int x, int y, unsigned int nglyp pglyph = FONTGLYPHBITS(pglyphBase, pci); gWidth = GLYPHWIDTHPIXELS(pci); gHeight = GLYPHHEIGHTPIXELS(pci); - if (gWidth && gHeight) { + if (gWidth > 0 && gHeight > 0) { nbyGlyphWidth = GLYPHWIDTHBYTESPADDED(pci); nbyPadGlyph = BitmapBytePad(gWidth); ===================================== xkb/maprules.c ===================================== @@ -470,15 +470,19 @@ CheckLine(InputLine * line, static char * _Concat(char *str1, const char *str2) { - int len; + size_t len; + char *tmp; if ((!str1) || (!str2)) return str1; + len = strlen(str1) + strlen(str2) + 1; - str1 = realloc(str1, len * sizeof(char)); - if (str1) - strcat(str1, str2); - return str1; + tmp = realloc(str1, len); + if (!tmp) + return str1; + + strcat(tmp, str2); + return tmp; } static void View it on GitLab: https://salsa.debian.org/xorg-team/wayland/xwayland/-/compare/af4b3d7250366e1a3822919e41fcf66da4966af2...c5a47fda896aeefbf1d06a73e392a294344f9e1a -- View it on GitLab: https://salsa.debian.org/xorg-team/wayland/xwayland/-/compare/af4b3d7250366e1a3822919e41fcf66da4966af2...c5a47fda896aeefbf1d06a73e392a294344f9e1a You're receiving this email because of your account on salsa.debian.org. Manage all notifications: https://salsa.debian.org/-/profile/notifications | Help: https://salsa.debian.org/help