# ext/gd: drop XPM support on Windows - Thread: https://externals.io/message/125502 - Original archive: https://news-web.php.net/php.internals/125502 - Started by Christoph M. Becker on 2024-09-11T12:55:08+00:00 - Messages: 9 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #125502 — Christoph M. Becker — 2024-09-11T12:55:08+00:00 URL: https://externals.io/message/125502#125502 Hi all, I'm in the progress of updating all libraries required for ext/gd on Windows. Since libxpm hasn't been updated for quite a while (we're still shipping libxpm 3.5.12), I've attempted updating to libxpm 3.5.17. However, besides the already existing mess of needing to fetch several X11 header files from other repos, I've noticed that support for FOR_MSW builds has completely been dropped[1]. That makes it even harder to have a somewhat clean build. Looking a bit further, I've noticed that three vulnerabilites have been fixed in libxpm 3.5.15[2]; the third one doesn't affect our builds, but the first two likely do, causing potential DoS, if crafted XPM images are read by imagecreatefromxpm() (but not by imagecreatefromstring() since this doesn't support XPM). While it should be possible to upgrade to libxpm 3.5.15 (or at least to backport the respective fixes), I don't think it makes sense to move forward supporting XPM images with ext/gd on Windows. Besides that this format is typically used on Linux, it is grossly out-dated. Even Gif is way superior, let alone PNG. Therefore I suggest dropping XPM support from ext/gd on Windows as soon as possible (might be a bit late for PHP 8.4, but might still be a good idea). Note that XBM support is unrelated, since this is handled by the bundled libgd without relying on any library. Also note that `getimagesize()` is also not affected, since it doesn't support XPM anyway. Any objections, or general thoughts? [1] [2] Christoph --- ## #125507 — Niels Dossche — 2024-09-11T15:38:20+00:00 URL: https://externals.io/message/125502#125507 · In reply to: #125502 > Hi all, > > I'm in the progress of updating all libraries required for ext/gd on > Windows. Since libxpm hasn't been updated for quite a while (we're > still shipping libxpm 3.5.12), I've attempted updating to libxpm 3.5.17. > However, besides the already existing mess of needing to fetch several > X11 header files from other repos, I've noticed that support for FOR_MSW > builds has completely been dropped[1]. That makes it even harder to > have a somewhat clean build. > > Looking a bit further, I've noticed that three vulnerabilites have been > fixed in libxpm 3.5.15[2]; the third one doesn't affect our builds, but > the first two likely do, causing potential DoS, if crafted XPM images > are read by imagecreatefromxpm() (but not by imagecreatefromstring() > since this doesn't support XPM). While it should be possible to upgrade > to libxpm 3.5.15 (or at least to backport the respective fixes), I don't > think it makes sense to move forward supporting XPM images with ext/gd > on Windows. Besides that this format is typically used on Linux, it is > grossly out-dated. Even Gif is way superior, let alone PNG. > > Therefore I suggest dropping XPM support from ext/gd on Windows as soon > as possible (might be a bit late for PHP 8.4, but might still be a good > idea). Note that XBM support is unrelated, since this is handled by the > bundled libgd without relying on any library. Also note that > `getimagesize()` is also not affected, since it doesn't support XPM anyway. > > Any objections, or general thoughts? > > [1] > > [2] > > Christoph I agree, let's reduce that maintenance burden. Kind regards Niels --- ## #125508 — Pierre Joye — 2024-09-11T16:04:28+00:00 URL: https://externals.io/message/125502#125508 · In reply to: #125502 hi Christoph, > Hi all, > > > Therefore I suggest dropping XPM support from ext/gd on Windows as soon > as possible (might be a bit late for PHP 8.4, but might still be a good > idea). > Given it is only useful on OSes relying on X/freedesktop, the impact should be zero. best, --- ## #125515 — Christoph M. Becker — 2024-09-11T22:03:46+00:00 URL: https://externals.io/message/125502#125515 > The GD PHP extension defines "imagecreatefromxpm" functions. How do you > manage it if XPM support is disabled? Oh, wow, gd/config.w32 needs to be fixed (anyway). As is, if libxpm is not available, ext/gd can't be built, what makes no sense, since the code which needs libxpm is already guarded by HAVE_GD_XPM. > GD (libxpm) must be updated for the currently supported PHP versions to > provide a security fix. Yeah, although I'm not really concerned about this, since I consider it highly unlikely that any PHP code running on Windows accepts XPM images from untrusted sources. > I have applied 5 patches on the master branch of winlibs/libxpm on > php-win-ext fork [1] and tagged "libxpm-3.5.12-1". The patch for the > last CVE has been manually integrated. > > How do we integrate this fix? Patch on winlibs-builder? PR on > winlibs/libxpm repository? This should be fixed in winlibs/libxpm. The patch in winlib-builder doesn't make sense; I think I did this to make it easier to update libxpm, though in hindsight this was probably a bad idea. Christoph --- ## #125516 — Derick Rethans — 2024-09-11T22:10:43+00:00 URL: https://externals.io/message/125502#125516 >Thank you for this suggestion. I have one question. > >The GD PHP extension defines "imagecreatefromxpm" functions. How do you manage it if XPM support is disabled? The function should not exist in that case, probably commented out through an #ifdef. cheers Derick --- ## #125518 — Pierre Joye — 2024-09-12T04:46:02+00:00 URL: https://externals.io/message/125502#125518 · In reply to: #125515 hi, > > > This should be fixed in winlibs/libxpm. The patch in winlib-builder > doesn't make sense; I think I did this to make it easier to update > libxpm, though in hindsight this was probably a bad idea. > I mentioned that long ago, but a maybe faster way to get windows ready sources is vcpkg: https://vcpkg.roundtrip.dev/ports/libxpm only changes required are for static lib names, as we use _a to differentiate. lib for dll or static links. best. --- ## #125521 — Christoph M. Becker — 2024-09-12T09:42:17+00:00 URL: https://externals.io/message/125502#125521 · In reply to: #125518 > I mentioned that long ago, but a maybe faster way to get windows ready > sources is vcpkg: > > https://vcpkg.roundtrip.dev/ports/libxpm > > only changes required are for static lib names, as we use _a to > differentiate. lib for dll or static links. Dropping our own builds in favor of some package manager is a worthwhile goal, but might be much harder than it seems. There was a discussion about CMake builds[1] a few years ago, and vcpkg and Conan have been shortly discussed there. The OP even started to provide some PRs regarding adding further libraries to Conan, but that apparently came to nothing. I can't remember if I ever had a closer look at Conan, but when I was looking into vcpkg a while ago, I've noticed a couple of problems: besides some missing packages (even for bundled extensions), and the naming conventions you've mentioned, maybe the biggest problem was their build style, namely to package a library with all it's dependencies. We would need to remove the dependencies after building and packaging to be able to stick to our distribution style. It seems to me that vcpkg is a great tool for applications, but not necessarily suitable for for us. [1] Christoph --- ## #125522 — Christoph M. Becker — 2024-09-12T09:48:11+00:00 URL: https://externals.io/message/125502#125522 · In reply to: #125515 > > […] > > Oh, wow, gd/config.w32 needs to be fixed (anyway). As is, if libxpm is > not available, ext/gd can't be built, what makes no sense, since the > code which needs libxpm is already guarded by HAVE_GD_XPM. Just for the record, that is supposed to be fixed with . Christoph --- ## #125523 — Pierre Joye — 2024-09-12T13:00:07+00:00 URL: https://externals.io/message/125502#125523 · In reply to: #125521 > > > > […] > > Dropping our own builds in favor of some package manager is a worthwhile > goal, but might be much harder than it seems. > sorry, I was not asking to use vcpkg, it has been discussed before as you remind us in this reply :) But the src are windows build ready and many of our less maintained deps are maintained there. best,