Use the new gimp_get_pdb_status() to forward the error returned by
gimp_file_load(). Previous code was always returning
GIMP_PDB_EXECUTION_ERROR when the file load was failing, but this was
not granular enough. In particular when the file load is actually
interactively cancelled through Esc or the "Cancel" button, we don't
want to display an error message on screen. Therefore we forward the
actual error raised by the underlining plug-in.
While we are at it, let's just add RGBE exporting. It's just as easy.
Also rename s/file-gegl-load-rgbe/file-load-rgbe/. All formats just use
the "file-FORMAT-(load|save)" naming for their procedure, even when
implemented directly through "gegl:load|save".
It seems that various software use something different after the "#?",
and even Blender code just ended up only use these 2 characters as magic
number. See also bug 792453.
Various plug-ins exporting metadata should now follow preferences, which
would override any default. Of course these preferences can still be
overriden by saved settings (global parasite), previous run settings,
and finally through the GUI when interactive.
This is a privacy concern. Whereas importing metadata is usually a good
idea, exporting it should be a conscious action. A lot of private data
can be leaked through metadata and many people don't realize it (which
also usually means they don't need it). On the other hand, the people
who realize it are the ones who would explicitly edit the metadata and
check what they want to be exported or not.
This is only a first step. Some people may want to always export the
metadata and for these people, there should be abilities to change the
default.
When running strtok() the first time, it needs to be non-NULL so we must
check for the string. This is even more important because NULL actually
has a special meaning in strtok() to indicate further search on the same
string, in a stateful way. So searching with NULL at first call was
crashing the metadata editor plug-in in my case.
I could also imagine it could have reused strings from previous
searches, mixing metadata contents in some edge cases. Anyway that would
be bad as well!
While I was there, I also checked for non-null search string before
strstr() calls, when there was not already such a check before. This
function also requires non-NULL haystack argument.
It feels like this code doesn't do much validity checks, and it's likely
there are more similar issues. I haven't reviewed the whole code, only
this part which was crashing here.
... the python console.
It was using "selected text", which is most often inverted color (close
if not identical to the background color). As a consequence, it made
stdout output unreadable by default, forcing themes to always define a
style for the python console. Using "normal text" is a much better
choice to default to something readable from a parent style.
As a consequence, I also removed "python-fu-console" styling from the
System theme, where there should be as few theming as possible.
After discussing with Mitch and understanding better the X bitmap/pixmap
history, I make the warning more specific to X bitmap cursors only (not
pixmap).
Also I can see our code always exports RGBA data, so I am not quite sure
if this warning even makes sense since X bitmaps are bicolor maps. On
the other hand, Mitch tells me that "these days gdk turns pixbufs into
bitmaps if the x server doesn't support rgba cursors", so maybe that can
still be of use to warn cursor designers for max compatibility.
Still that's pretty old compatibility stuff, so let's replace "may" by
"might".
Leading 0s have no special value, we use base 10 anyway. Removing
leading 0s allows to not trigger the 8-digit test, hence modify a valid
cursor size unecessarily.
We were basing our max export size on a macro value defined in
libXCursor code: MAX_BITMAP_CURSOR_SIZE. This macro is still defined in
libXCursor and still has the same value (64), yet it is unsure how far
or even where this is enforced since it seems we can get at least 96px
cursors in GNOME/X11.
As a consequence, this commit:
- still warns when cursor size is over this value, with more explicit
text, yet does not change the cursor size anymore! So it is now
possible to export bigger cursors, but you still get a warning.
- only changes the cursor size for the existing more-than-8-digits test
and I add a warning when it does so (we should never modify an image
silently!).
- adds the size 96 as not triggering the warning about GNOME Settings
since it definitely looks like this size is valid there (according to
my own empirical tests). Also since 96 is higher than the libXCursor
current MAX macro value, this really raises the question to where this
max is enforced and whether we should not just drop the first warning.
Note that it breaks a bit the string freeze since I modify one string
and adds one. Sorry for this!
After commit f51acf3bfb the python console no longer
initialized gimpui, because it is no longer part of module
initialization.
If the plug-in is run noninteractively and it imports
gimpui explicitely it is now necessary to call gimp_ui_init ()
at the right time
Load as much of a broken/truncated JPEG as possible:
As soon as loading the scanlines has started, set a new setjmp()
location that doesn't abort loading alltogether but keeps the loaded
part of the image.
It is possible to trigger a heap overflow with insanely large GBR
files with a deprecated file format on 32 bit systems.
The problem is that old versions of GBR allowed an additional pattern
after the brush data. These patterns have always 4 bytes per pixel,
but the initial size check is performed with the bytes per pixel of
the brush, which can be different.
If the brush has 1 byte per pixel and the dimensions are sufficiently
large, this can trigger a heap overflow with attacker-controlled
amount and content of data.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
...mergedimage.png
fix also the thumbnail creation, that:
must be 8 bit; should not be upscaled; should not have frame
or decoration, that I interpreted as when there are transparent
areas they should not be blended with a background color,
so instead of image_flatten use image_merge_visible_layers
(also for the merged_image)
fopen() modes "wb+" and "w+b" are aliases of the same opening mode
(truncate/create in binary read/write). But it turns out that Windows
implementation does not understand "wb+". The alias "w+b" works fine in
my tests.
There should be absolutely no code change semantic in this commit. If
there is, that's a mistake and it's on me. I only cleaned up the syntax
in the C file which basically was following none of GIMP coding style
(tabs everywhere, brackets at end of lines, nearly no space anywhere so
all the code was a compressed mess which was hard to read, indentation
absolutely wrong everywhere, etc.).
I cleaned it up with a bunch of regexp search-and-replace followed by a
lot of manual cleaning and verification as well.
I also tested with various FLI files found on the web, and they were
loading fine in GIMP. So I believe/hope that I didn't mess up somewhere,
but it looks as the cleaning went all fine.
It is possible to trigger a heap overflow while parsing FLI files. The
RLE decoder is vulnerable to out of boundary writes due to lack of
boundary checks.
The variable "framebuf" points to a memory area which was allocated
with fli_header->width * fli_header->height bytes. The RLE decoder
therefore must never write beyond that limit.
If an illegal frame is detected, the parser won't stop, which means
that the next valid sequence is properly parsed again. This should
allow GIMP to parse FLI files as good as possible even if they are
broken by an attacker or by accident.
While at it, I changed the variable xc to be of type size_t, because
the multiplication of width and height could overflow a 16 bit type.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
This reverts commit 06d24a79af.
The CVE is still fixed but now in a different way. Commit 4fa0cd4dcf
passes instead the accurate string length when using the string, hence
making it work even when not NUL-terminated. This has the advantage of
having the GBR file loaded in the end, despite such file format error. I
am personally not persuaded this is the best path since a file with such
an error may either be corrupted, or worse may have been constructed on
purpose to be harmful, so rejecting it directly may be the safe choice.
Nevertheless I may also be too doubtful and maybe trying to save a
slightly corrupted file may be the nicest choice indeed.
The file formats GBR and PAT contain names which are supposed to be
NUL-terminated within the files. If no such terminating NUL byte
exists, the parsers of GBR and PAT trigger an out of boundary read
during utf-8 conversion.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Some g_message() stay when they are used as debug or warning message
(without actually cancelling load action). But all fatal errors now use
g_set_error().
Jehan