Deprecate the BMP texture file format

[erlehmann]

Problem

I am unaware of any usage of BMP in Minetest besides builtInFont.bmp in IrrlichtMT.

Therefore, I want to discuss here removing the support for BMP from Minetest.

Rationale: Where any input is processed we have an opportunity for security bugs.

The recent fuzzing of the TGA decoder has revealed several bugs, that have since been fixed thanks to the quick response of Irrlicht upstream. Bugs are not unique to Irrlicht's handling of the TGA file format though: libpng has a long list of vulnerabilities, libjpeg turbo has vulnerabilities and libwebp was so bad that one could exploit many applications by loading a single image file.

The IrrlichtMT BMP decoder code is larger than the TGA decoder code and from a cursory look seems less useful than all other texture formats that Minetest uses (JPEG, PNG, TGA), for any purpose:

• Compared to JPEG: BMP has no lossy compression.
• Compared to PNG: BMP probably implies larger files.
• Compared to TGA: BMP is more difficult to read/write.

I am not 100% sure about this assessment: The BMP decoder seems to support 1bpp bitmaps, which might make it useful for storing black-white textures in a lower amount of space than other formats – but I also doubt that such things are important for Minetest if BMP is indeed not used in mods, games or texture packs.

Solutions

I propose the following plan:

1. Figure out if the BMP loader is as useless as I suspect. Maybe @mzeilfelder knows something?
2. Research if BMP files are contained in mods, games or texture packs that can be found on:
   a) ContentDB
   b) Minetest forums
   c) GitHub, GitLab, MeseHub, Codeberg and other forge sites.
3. Research if any BMP files are written (e.g. to files or node/item metadata) by mods or games that can be found.
4. Research if any BMP files are read (e.g. from files or node/item metadata) by mods or games that can be found.

If not one mod can be found that uses BMP in a way that the engine must support it, remove it soon. Otherwise do this:

5. Deprecate BMP as soon as possible.
6. Help to patch all Minetest-adjacent software that anyone uses to no longer need BMP support.
7. Remove BMP.

Alternatives

• Do nothing.
• Fuzz the BMP decoder and report all bugs to Irrlicht upstream, then merge the changes.

Additional context

Mods on ContentDB that contain the string bmp – probably no engine support necessary

@rollerozxa did a ContentDB zipgrep searches:

• ContentDB zipgrep search for the string ”bmp”
• ContentDB zipgrep search for the string “BMP”
• ContentDB zipgrep search for the string “bitmap”

This yielded:

• 4 copies of modlib code that uses file endings to collect all media in a mod in:
  • nonsensical_skyblock
  • modlib
  • spiraling_down
  • survivetest
• arena_lib code that finds unused JPEG, PNG, BMP (but not TGA) files
• minebase_core can determine image dimensions for several image formats, among them BMP

Mods on ContentDB that contain BMP files – so far all have been unpublished

• juanchi from @runsy (status unclear)
• minetest_life from @garywhite207 used to contain ./mods/pipeworks/textures/New Bitmap Image.bmp but that file was deleted in 2016

Mods that read BMP files to place nodes in the world – probably no engine support necessary

• bmpmap from @ch98-1 via https://forum.minetest.net/viewtopic.php?t=8374 (2013 - 2015)
  • This mod reads a BMP file to use it for an elevation-based mapgen.
• growwall from @dgm3333 via https://forum.minetest.net/viewtopic.php?t=11565 (2014 - 2015)
  • This mod reads a BMP file as a template to put nodes into the world. Does it display a BMP to the user though?
• minetest-imageloader from @kaeza via https://forum.minetest.net/viewtopic.php?t=6751 (2013 - 2017)
  • This mod loads BMP files (only 24bpp). It seems to be a library.
  • @HybridDog used minetest-imageloader to put a comic girl in a Minetest world. (2014)
  • @bobombolo used minetest-imageloader to put a photo of a person in a Minetest world (2015).
  • @eidy used minetest-imageloader to a photo of Donald Trump in a Minetest world (2017)
  • Minetest forum user slopsbucket used minetest-imageloader to help building Angkor Wat in Minetest (2015)
• realterrain from @bobombolo via https://forum.minetest.net/viewtopic.php?f=9&t=12666 (2015 - 2023)
  • This mod makes impressive landscapes from BMP files that it reads in. It also reads 16 bit PNGs and TIFF though?

[rollerozxa]

Worth noting that the Irrlicht built-in font is a BMP image, but this can be solved by just converting it into a PNG image and embedding it into the header (see Voxelmanip-Classic/irrlicht-vmc@7a96c42).

[Wuzzy2]

I'm normally very sensitive when it comes to breaking backwards-compability but ... BMP support? Yeah, good riddance. I will not shed a single tear for BMP. Any mod depending on it really had it coming anyway. I wasn't even aware BMP is supported anyway.

And if this helps freeing up valuable developer time for more important things, I'm all for it.

[erlehmann]

@ch98-1 @dgm3333 @kaeza @HybridDog @bobombolo @eidy does any one of you rely in their code on the ability of the Minetest engine to display BMP images? I looked a bit at what your mods do and it seems that this is not the case. But I might be wrong, so a confirmation from the authors that there is no breakage if BMP is removed would be great.

@rollerozxa embedding a binary into a header file looks like a source code crime. Why not load it from a file at build time?

@Wuzzy2 I understand that you might not miss BMP, but others might (I will not though) and if removing BMP creates issues for them is what I am trying to find out with this issue. People are often not aware of features that they do not use, so “I do not know this was even supported” is rarely a good justification for removal or just deprecation, Not having any users who rely on a feature though, may be a good justification – the Linux kernel for example may remove support for some old WLAN hardware soon, with a stated plan to add in back any driver that even one user complains about being missing (as it means they have/use the hardware).

Edit: I do not know if you meant “Any mod depending on it really had it coming anyway.” as a dark joke (like this broadway version of “Cell Block Tango” does), but that one in particular seems like victim-blaming to me. Using a feature of a software that has worked for years is not “having it coming”, regardless of how shitty that feature looks.

Regarding the valuable developer time, I doubt that texture loading code is a big maintenance burden in normal development. It does, however, constitute attack surface and therefore should be minimized or removed and made as good as possible: Deleted code is debugged code.

[rubenwardy]

On ContentDB, the only two packages containing .bmp files are minetest_life and juanchi - both of which are unpublished

[rubenwardy]

For reference, there are nine published packages that contain tga files: refi_textures, mineclone2, xmaps, tga_encoder, mineclonia, naturalslopes_minetest_game, painting, wac, petz

(not including devtest)

[mzeilfelder]

I don't think there is a problem replacing the build-in font with a png. It's a header file, because there is no build-system independent way to load resource files in c++. And the main reason why that specific font is still used in Irrlicht is that it contains some icons for GUI elements (I think for checkboxes and scrollbars, maybe more).

[erlehmann]

On ContentDB, the only two packages containing .bmp files are minetest_life and juanchi - both of which are unpublished

About minetest_life

I downloaded minetest_life from the link here https://content.minetest.net/packages/garywhite/minetest_life/releases/ which resolves to https://content.minetest.net/packages/garywhite/minetest_life/releases/1130/download/ and the only file I could find was ./mods/pipeworks/textures/New Bitmap Image.bmp which is a 512×512 24bpp (RGB) image which is mostly white, except for 4 pixels somewhere in the middle … that nevertheless takes up 772k of disk space. Which is at least a bit weird, because the image contains only grayscale colors, i.e. even with 1 byte per pixel and uncompressed it should only take up about third the size.

I guess my joke about software being made “by the creators of ‘New Folder (3)’” has finally become true.

Anyway, not only is minetest_life unpublished, the file New Bitmap Image.bmp has been removed from minetest_life in this commit in 2016: garywhite207/minetest_life@6134c28

About juanchi

juanchi has not only been unpublished, but also deleted from GitHubl. @runsy may we assume that juanchi is obsolete?

[erlehmann]

For reference, there are nine published packages that contain tga files: refi_textures, mineclone2, xmaps, tga_encoder, mineclonia, naturalslopes_minetest_game, painting, wac, petz

(not including devtest)

To elaborate, at least the following packages write TGA bitmaps at runtime (engine support means those can be displayed):

• mineclone2 (via mcl_maps → tga_encoder)
• mineclonia (via mcl_maps → tga_encoder)
• ucsigns (via unicode_text → tga_encoder)
• painting (via tga_encoder)
• xmaps (via tga_encoder)

Amusingly, the map mods that write TGAs are exactly the opposite from the way BMP has been used in Minetest mods (making bitmaps from nodes vs making nodes from bitmaps).

@rubenwardy given that both tga_encoder and ucsigns do not contain TGA bitmaps but generate them (with ucsigns even stuffing them in node meta) and you found tga_encoder but not ucsigns … what exactly was your search query to find BMP and TGA usage here? I wonder if we are missing some mods that generate BMP at runtime (those would need to be patched if possible).

[rubenwardy]

I searched for file names containing bmp/tga, it won't find dynamic uses

[rollerozxa]

I made a search for just "bmp" in code of packages on ContentDB: https://content.minetest.net/zipgrep/2784bb41-d88b-46e7-9d27-641dd3b9abb3/ (archived link)

Only results appear to be for lists of valid texture filename extensions, mostly originating from modlib.

Edit: Here are also the zipgrep results för "BMP" (archive) and "bitmap" (archive) too, though mostly seems to be false positives apart from a function that can read the resolution of a BMP image (in addition to a number of other formats, some of which we don't support either)

[erlehmann]

@appgurueu @Zughy @gabri-viane your modlib and arena_lib and minebase_core code will not break at all if BMP support is removed, is that correct?

[sfan5]

We really don't have to go and ping every mod author on GH and ask them if they are fine with it.
We can just deprecate it, leave it for a few releases and if nobody has complained by then actually remove it.

[erlehmann]

@sfan5 my rationale here is that as supporting BMP is possibly security-relevant (did you fuzz the Irrlicht BMP loader too by any chance?), I would like it to be removed as soon as possible (if possible at all). If nothing is broken by removing BMP (which seems to be the case, but I am not done with the research yet), ”as soon as possible” might be ”as soon as the next security vulnerability is found in the loader” or maybe even earlier.

[Zughy]

Go with it, never touched a .bmp on Minetest (unsubscribing)

[appgurueu]

Yes, modlib will not break; it only uses the .bmp file extension to collect media (so if Minetest removes BMP support, the worst case is that modlib "recognizes" media which Minetest does not (which is typically not an issue) as I forget to update the file extension list, which it seems I already did - Minetest doesn't support .wal, .ppx etc. anymore).

modlib doesn't touch BMPs besides that.

Feel free to go ahead with deprecating / removing BMP.

[HybridDog]

minetest-imageloader uses its own BMP decoder, so we can ignore it for this issue.

I also want to get rid of BMP developed by Microsoft Corporation.

[grorp]

Done by #13922.