This hopefully fixes the raster-source test case crashing:
cairo-svg-surface.c:2269: _cairo_svg_surface_emit_pattern: Assertion `!"reached"' failed.
I cannot / did not test this change locally and rely on CI to tell me
whether this works.
Signed-off-by: Uli Schlachter <psychon@znc.in>
This should allow to use them for allocating large amounts of memory.
Also use explicit checks for zeros to not make the compiler think that it is a boolean context.
This function parses some raw font data and it trusts the font to be
well-formed. This means that a font can just say "this segment is a
gigabyte large" and the code will happily jump ahead in memory. Bad
things then happen in practice.
Fix this by adding lots of bounds check.
Also, an existing bounds check makes sure we are still before the end of
the data, but then happily reads the next six bytes. Fix this by making
sure we actually have six bytes of data.
No regression test since the last few times I tried to do this for font
issues, I ended up with a large/huge blob of font data. Too large for
the test suite.
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=27969
Signed-off-by: Uli Schlachter <psychon@znc.in>
Tested with valgrind. Before this patch, I got the following "definitely
lost" entry, which is gone afterwards:
94,416 bytes in 1 blocks are definitely lost in loss record 427 of 427
at 0x483877F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4B053F8: cairo_truetype_font_write_glyf_table (cairo-truetype-subset.c:625)
by 0x4B06219: cairo_truetype_font_generate (cairo-truetype-subset.c:991)
by 0x4B06917: cairo_truetype_subset_init_internal (cairo-truetype-subset.c:1159)
by 0x4B06D72: _cairo_truetype_subset_init_pdf (cairo-truetype-subset.c:1255)
by 0x4B6B113: _cairo_pdf_surface_emit_truetype_font_subset (cairo-pdf-surface.c:5892)
by 0x4B6C2AD: _cairo_pdf_surface_emit_unscaled_font_subset (cairo-pdf-surface.c:6366)
by 0x4B02FC7: _cairo_sub_font_collect (cairo-scaled-font-subsets.c:741)
by 0x4B03A7A: _cairo_scaled_font_subsets_foreach_internal (cairo-scaled-font-subsets.c:1062)
by 0x4B03B21: _cairo_scaled_font_subsets_foreach_unscaled (cairo-scaled-font-subsets.c:1090)
by 0x4B6C3ED: _cairo_pdf_surface_emit_font_subsets (cairo-pdf-surface.c:6412)
by 0x4B62B1A: _cairo_pdf_surface_finish (cairo-pdf-surface.c:2222)
To reproduce, run the test case from the below link.
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28023
Signed-off-by: Uli Schlachter <psychon@znc.in>
The documentation for _cairo_array_append_multiple says "one or more items".
If it is called with num_elements=0, it ends up calling _cairo_array_grow_by
with num_elements=0, which if the array is currently empty (as here) leads
to undefined behavior in _cairo_array_allocate in the line
*elements = array->elements + array->num_elements * array->element_size;
because it ends up trying to add 0 to a null pointer. C doesn't allow this.
(UBSan flags this as "applying zero offset to null pointer".)
The error paths in _cairo_pdf_interchange_begin_dest_tag() do not clean
up and cause some memory to be leaked. Fix this by adding the necessary
free()s.
The first hunk, the missing free(dest) was found by oss-fuzz (see link
below).
The second hunk is an obvious follow up. It also cleans up the memory
allocated by _cairo_tag_parse_dest_attributes().
The cleanup in the second hunk is similar to the function
_named_dest_pluck() in the same function, but that function also removes
the entry from a hash table. The error case here is that exactly this
hash table insertion failed. Thus, the code cannot simply use the
already existing function.
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=30880
Signed-off-by: Uli Schlachter <psychon@znc.in>
A hash value is encoded in base 26 with upper case letters for font
names.
Commit ed984146 replaced "numerator = abs (hash);" with "numerator =
hash;" in this code, because hash has type uint32_t and the compiler
warned about taking the absolute value of an unsigned value. However,
abs() is actually defined to take an int argument. Thus, there was some
implicit cast.
Since numerator has type long, i.e. is signed, it is now actually
possible to get an overflow in the implicit cast and then have a
negative number. The following code is not prepared for this and
produces non-letters when encoding the hash.
This commit fixes that problem by not using ldiv() and instead using /
and % to directly compute the needed values. This gets rid of the need
to convert to type long. Since now everything works with uint32_t, there
is no more chance for negative numbers messing things up.
Fixes: https://gitlab.freedesktop.org/cairo/cairo/-/issues/449
Signed-off-by: Uli Schlachter <psychon@znc.in>
When a recording surface with non-zero origin is
saved to a png file, it gets cut off. Fix this by
setting a device offset when acquiring the source
image.
asan was complaining that the limits struct goes out
of scope before it is used via the pointer in the polygon struct,
and it is right:
==386746==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7ffd3ccebdfc at pc 0x7f783d5eaaee bp 0x7ffd3cceba80 sp 0x7ffd3cceba70
READ of size 4 at 0x7ffd3ccebdfc thread T0
#0 0x7f783d5eaaed in _add_clipped_edge ../src/cairo-polygon.c:351
#1 0x7f783d5ebba3 in _cairo_polygon_add_edge ../src/cairo-polygon.c:520
#2 0x7f783d5ebc82 in _cairo_polygon_add_external_edge ../src/cairo-polygon.c:530
#3 0x7f783d582149 in _cairo_filler_line_to ../src/cairo-path-fill.c:63
#4 0x7f783d588d9c in _cairo_path_fixed_interpret ../src/cairo-path-fixed.c:831
#5 0x7f783d582a44 in _cairo_path_fixed_fill_to_polygon ../src/cairo-path-fill.c:147
#6 0x7f783d6204fe in _cairo_spans_compositor_fill ../src/cairo-spans-compositor.c:1151
#7 0x7f783d5126de in _cairo_compositor_fill ../src/cairo-compositor.c:203
#8 0x7f783d5571f9 in _cairo_image_surface_fill ../src/cairo-image-surface.c:1003
#9 0x7f783d647f2f in _cairo_surface_fill ../src/cairo-surface.c:2424
#10 0x7f783d52ebea in _cairo_gstate_fill ../src/cairo-gstate.c:1312
#11 0x7f783d51cca4 in _cairo_default_context_fill ../src/cairo-default-context.c:1057
#12 0x7f783d6812d6 in cairo_fill ../src/cairo.c:2421
GTK has a testcase that tests the error when creating
an oversize image, and asan tells me that it triggers
a memory leak in cairo:
Direct leak of 160 byte(s) in 1 object(s) allocated from:
#0 0x7f1122755667 in __interceptor_malloc (/lib64/libasan.so.6+0xb0667)
#1 0x7f1120cc83e8 in _cairo_pattern_create_solid ../src/cairo-pattern.c:607
#2 0x7f1120cc8487 in _cairo_pattern_create_in_error ../src/cairo-pattern.c:630
#3 0x7f1120cc87cb in INT_cairo_pattern_create_for_surface ../src/cairo-pattern.c:736
#4 0x7f1120c1f1c7 in _cairo_default_context_set_source_surface ../src/cairo-default-context.c:327
#5 0x7f1120d8386a in INT_cairo_set_source_surface ../src/cairo.c:982
#6 0x7f1121d005a2 in gdk_cairo_set_source_pixbuf ../gdk/gdkcairo.c:234
#7 0x401427 in test_set_source_big_pixbuf ../testsuite/gdk/cairo.c:23
If CAIRO_ANTIALIAS_DEFAULT is selected and system is set to subpixel, it
is perfectly normal to switch to CAIRO_ANTIALIAS_SUBPIXEL.
But when the calling application specifically requested
CAIRO_ANTIALIAS_GRAY then Cairo should honor the request.
This is an issue we have had for years in GIMP, where text on images
would render differently depending on the system the file is opened on.
This is obviously not right as a graphics work should be system
independant and allow the creator to decide if one wants a text to have
grayscale or subpixel rendering (a settings which would stick when
sharing the work file). Cairo should not override this.
The CAIRO_ANTIALIAS_DEFAULT settings exists exactly for this (when we
want Cairo to choose for us, in a system-dependant way); other settings
are when we need system independance.
Thanks to Adam Fontenot for initial investigations and tests on this and
other contributors before this.
The expression &image_surface->base basically just casts the
cairo_image_surface_t* to cairo_surface_t*. However, technically it is a
NULL pointer dereference and UndefinedBehaviorSanitizer flags it as
such:
runtime error: member access within null pointer of type 'cairo_image_surface_t' (aka 'struct _cairo_image_surface')
This commit fixes this by adding a NULL check.
Signed-off-by: Uli Schlachter <psychon@znc.in>
-----BEGIN PGP SIGNATURE-----
iQJOBAABCgA4FiEEpmEQCz2sHU8srYpU5gOyV4+48PsFAl/Cz0AaHGJyeWNlQGJy
eWNlaGFycmluZ3Rvbi5vcmcACgkQ5gOyV4+48PtUXxAAnYipgwpcIKWoSt4eP1o/
SPqeupY0AQUWB8y6xKoAhcBzt/HOQBqMxWo+zJSkIndJMRusezSmnt+qHY1bXGTX
pqyipyYfSUJnT6BB/iZvwcAPlW2ISLuI85qdW126nKX9jxCx1uTejogGzeLC2nLK
DucPZ1N7HU87GEc2mcl1aYkwTHp1f4jEQhRFExvCJg9YA3W+SgDQ2XEMEeewqljM
/AT7tL3yWFCv4OU5ci7qhUZPp9ZgWZ1vc+0zS1MgpVo7XwxUWL+NaCbhbVJoH6Dq
rwPE+RJOh1zkkatY/jZQYmW65gnqxed7lcUJ8XmRCUQiIQNvKzYApthj1EW4bxV4
yxz+O6OiHlnnf20IwqAirEeHOUHYwINHVTE/UZovB1pu983iPQztNH9wfSmhDkhj
MS+mNVyLJc4Jb/UQvLOkDVljuT2tmtOYytSeMb5z0D+EjBPDAnWZk9RV7UouK+nF
HjuXEC45NEPzD5H4G8R4HZXBlSbV/SuzyS0Ljor8GBSdAJ9Xf6pDojgNCXLzHgny
byD1j7jB5rqmGfy72IRUtLFqThroEiTaKXPNJ44P9lK/YdpBZfn99BmtLEUETNeM
enm8ex1CWYrzLlq5UwM/2FYLE6Tiy5AQOqGfcnQpuaEG4qKao5Za7hmUy3yWitfa
OK/2GwlDbHXfu/Ekg1kzzFk=
=jna2
-----END PGP SIGNATURE-----
Merge tag '1.17.4'
cairo 1.17.4 release
We are not quite sure what happened, but the 1.17.4 tag and the master
branch have different Git histories, but the same content. The merge
commits are missing from the history of the tag. This merge is simply
the result of "git merge 1.17.4".
This fixes: https://gitlab.freedesktop.org/cairo/cairo/-/issues/446
X11 use uint16_t for the width/height of things. Anything too large will
be truncated when sending the request to the X11 server. This commit
adds a size check to a function that did not check things and then later
caused a segmentation fault.
Not adding a test case because the test case from the below bug report
allocates 3,5 GiB of memory, which I find too much for a test.
Fixes: https://gitlab.freedesktop.org/cairo/cairo/-/issues/414
Signed-off-by: Uli Schlachter <psychon@znc.in>
Surfaces from _cairo_surface_create_in_error() have no backend. This
commit fixes a NULL pointer dereference in cairo_win32_surface_get_dc().
Fixes: https://gitlab.freedesktop.org/cairo/cairo/-/issues/405
Signed-off-by: Uli Schlachter <psychon@znc.in>
In commit 10e58a4a I changed the code in cairo-surface.c to avoid
setting surface->is_clear = FALSE; in some situations where it was not
necessary, because the operation did not actually modify anything (it
returned CAIRO_INT_STATUS_NOTHING_TO_DO). However, that change
accidentally also caused _cairo_surface_paint() not to set
surface->is_clear = TRUE; in similar cases. That was unintended.
This commit fixes that by always setting is_clear = TRUE when necessary,
but keeps the optimisation of not setting is_clear = FALSE when not
necessary.
The connection to the below issue is that the issue happened with
surfaces with width=0. Clearing such a surface with CAIRO_OPERATOR_CLEAR
causes CAIRO_INT_STATUS_NOTHING_TO_DO and thus is_clear = TRUE was not
set. This error was later caught by a failed assertion.
Fixes: https://gitlab.freedesktop.org/cairo/cairo/-/issues/283
Signed-off-by: Uli Schlachter <psychon@znc.in>
An error in _cairo_surface_snapshot_copy_on_write() results in a
snapshot in an error state and the snapshot's ->target could now point
to a surface from _cairo_surface_create_in_error(). These surfaces e.g.
have ->backend == NULL. Thus, anything looking at ->backend->type now
explodes. This commit deals with two places which caused segfaults in
this situation.
There is no test case for this, because
_cairo_surface_snapshot_copy_on_write() really is not supposed to fail.
Found-while-investigating: https://gitlab.freedesktop.org/cairo/cairo/-/issues/448
Signed-off-by: Uli Schlachter <psychon@znc.in>
The code in cairo-cff-subset.c parses a binary format without seeming to
bother much with verifying the data. The result is that poppler can be
used to cause an out-of-bounds write in cairo_cff_font_read_fdselect()
via a crafted font file. Fix this by adding the needed length check.
The other code in the file also contains lots of similar things. Since I
cannot really fix everything properly, I'll just fix the one instance
that was found by a fuzzer.
No testcase is added, because this depends on a broken font that is
quite large. Adding something this big to the test suite does not seem
sensible.
Fixes: https://gitlab.freedesktop.org/cairo/cairo/-/issues/451
Signed-off-by: Uli Schlachter <psychon@znc.in>
The code was copying from the wrong member of an union. This caused a
huge num_dashes value to be read, which then caused a so large memory
allocation that malloc returned an error.
Fixes: https://gitlab.freedesktop.org/cairo/cairo/-/issues/448
Signed-off-by: Uli Schlachter <psychon@znc.in>
The code in cairo-cff-subset.c parses a binary font format without
seeming to bother much verifying the data. The result is that poppler
can be used to cause an out-of-bounds access in
cairo_cff_parse_charstring() via a crafted font file. Fix this by adding
the needed length check.
The other code in the file also contains lots of similar things. Since I
cannot really fix everything properly, I'll just fix the one instance
that was found by a fuzzer.
No testcase is added, because this depends on a broken font that is
quite large. Adding something this big to the test suite does not seem
sensible.
Fixes: https://gitlab.freedesktop.org/cairo/cairo/-/issues/444
Signed-off-by: Uli Schlachter <psychon@znc.in>
