Make sure surface->object_stream.stream is cleaned up even if things
failed
In poppler oss-fuzz tests we are getting this leak reported
Direct leak of 64 byte(s) in 1 object(s) allocated from:
#0 0x5747417eabd9 in __interceptor_calloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:74:3
#1 0x574742706f5b in _cairo_memory_stream_create cairo/src/cairo-output-stream.c:741:14
#2 0x5747426757b8 in _cairo_pdf_surface_open_object_stream cairo/src/cairo-pdf-surface.c:2307:34
#3 0x57474266b880 in _cairo_pdf_surface_finish cairo/src/cairo-pdf-surface.c:2700:14
#4 0x57474261afc6 in _cairo_surface_finish cairo/src/cairo-surface.c:1043:11
#5 0x57474261afc6 in cairo_surface_finish cairo/src/cairo-surface.c:1092:5
#6 0x57474270808a in _cairo_paginated_surface_finish cairo/src/cairo-paginated-surface.c:215:2
#7 0x5747426175c2 in _cairo_surface_finish cairo/src/cairo-surface.c:1043:11
#8 0x5747426175c2 in cairo_surface_destroy cairo/src/cairo-surface.c:978:2
This fixes it.
_cairo_pdf_surface_finish was succeeding past
_cairo_pdf_surface_open_object_stream that allocates surface->object_stream.stream,
failing when calling _cairo_pdf_surface_emit_font_subsets
and that memory was never freed
To avoid any possibility of uninitialized memory.
The exceptions are:
- where the allocation is immediately overwritten by a memcpy or struct copy.
- arrays of structs to avoid any performance impact (except when the
array is returned by the public API).
Not all platforms handle a zero sized allocation in calloc the
same. This macro ensures that _cairo_calloc(0) always returns NULL
similar to _cairo_malloc(0).
- If a link has both 'dest' and 'uri', the 'dest' will be used if it
exists, otherwise it will fallback to using the 'uri'.
- Ensure that a missing 'dest' does not result in an error. Instead a
warning is printed if CAIRO_DEBUG_TAG is set, and a link to the
current location is embedded in the PDF. ie the link does
nothing. Cairo needs to embed a link even if no destination is
available because when links are embedded at the end of the
document, the content stream already contains link tags.
- Remove cairo_pdf_interchange_write_forward_links. This code was
originally used prior to !463 when cairo wrote the links at the end
of each page. Now the links are written at the end of the document
so there are no longer any forward links with an unknown
destination, unless the destination does not exist.
- When 'internal' is not used, use the 'dest' name to reference the
link. Ensure non ASCII names are correctly encoded.
Since a user-font might be calling stroke, and PDF has separate
stroke and fill colors.
Note that this bug was not exposed in Poppler. It's probably a
bug there. But multiple other viewers expoed this bug.
Fixes https://gitlab.freedesktop.org/cairo/cairo/-/issues/813
../src/cairo-pdf-surface.c: In function '_cairo_pdf_surface_open_content_stream':
../src/cairo-pdf-surface.c:2537:45: error: format not a string literal and no format arguments [-Werror=format-security]
2537 | str);
| ^~~
cc1: some warnings being treated as errors
../src/cairo-pdf-surface.c:2505:9: warning: variable 'status' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (surface->base.status != CAIRO_STATUS_SUCCESS)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/cairo-pdf-surface.c:2573:9: note: uninitialized use occurs here
if (status == CAIRO_STATUS_SUCCESS)
^~~~~~
../src/cairo-pdf-surface.c:2505:5: note: remove the 'if' if its condition is always false
if (surface->base.status != CAIRO_STATUS_SUCCESS)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/cairo-pdf-surface.c:2497:5: note: variable 'status' is declared here
cairo_status_t status, status2;
^
This implements Type 3 color fonts for PDF for any font with a
CAIRO_SCALED_GLYPH_INFO_RECORDING_SURFACE. This includes user-fonts,
SVG fonts, and COLR fonts.
Glyphs with foreground colors are not yet implemented as Type 3 glyphs
and will be rendered as images by cairo-surface.
The problem is _cairo_recording_surface_replay_and_create_regions()
stores the cairo_recording_region_type_t in the same structure as the
recording commands. This does not work well when the recording surface
is used as source by multiple surfaces
Fix this by moving the cairo_recording_region_type_t into a separate
struct cairo_recording_regions_array_t. This struct is stored in a
list that allows multiple create regions results to be store in the
surface.
The new function _cairo_recording_surface_region_array_attach() is
used to create a new cairo_recording_regions_array_t, attach it to the
recording surface and return a unique region id.
The _cairo_recording_surface_replay_and_create_regions() and
_cairo_recording_surface_replay_region() functions use this region id
to identify the cairo_recording_regions_array_t.
To handle nested recording surfaces, when replaying a recording, the
region id is passed to the target as an extra parameter in the surface
pattern. The wrapper surface makes a temporary copy of the pattern to
ensure the snapshot pattern in the recording surface is not modified.
cairo_recording_regions_array_t has a reference count so the target
can hold on to the cairo_recording_regions_array_t after the paginated
surface has called _cairo_recording_surface_region_array_remove().
Before this commit, cairo_pdf_version_to_string() would return the
result of an out-of-bounds array access when called with a negative
value. This commit adds a check against this.
No unit test added since there are no tests for
cairo_pdf_version_to_string() that I could easily add such a test to.
Fixes: https://gitlab.freedesktop.org/cairo/cairo/-/issues/590
Signed-off-by: Uli Schlachter <psychon@znc.in>
PDF output uses q/Q operators to reset the clip path. This also resets
the PDF graphics state including the current blend mode.
When resetting the clip path, reset the current blend mode tracked by
the PDF surface so that if a non-default blend mode is required, the
surface will emit the correct blend mode.
Fixes#514
This allows all objects that were previously emitted uncompressed to
be compressed into a an object stream.
Currently only /Page, /Pages, and /Catalog have been converted to use
object streams.
Previously, forward references were required to use named destinations.
This patch is based on the patch in #336 by Guillaume Ayoub <guillaume.ayoub@kozea.fr>
that converted all links to indirect objects written at the end of the document.
I have reworked the patch so that only forward references to future page numbers are
written as indirect objects. Backward references and named destinations remain as they
are. This is to minimize the number of objects written to the PDF file.
Fixes#336
cairo-pdf was silently ignoring write errors in
_cairo_pdf_surface_finish(). Any write errors that happened here ended
up setting a "status" variable, but the value in this variable was then
unused.
This commit fixes this bug by passing this error on to the caller.
Additionally, this also adds a test case for this behaviour based on
writing to /dev/full. This file is non-standard and thus the test first
checks that this file exists and is writable before trying to write to
it.
This bug was found based on a report from Knut Petersen [0].
[0]: https://lists.cairographics.org/archives/cairo/2021-July/029281.html
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>
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>
From further testing and investigation it appears that many PDF viewers
already have a workaround to invert Adobe CMYK JPEGs, so our generated
PDFs display incorrectly with those viewers due to double-inversion.
Further investigation will be needed to find a better solution that
doesn't cause regression for some PDF viewers; perhaps PDF viewers that
lack this inversion workaround should be changed to include it. For now
we'll drop the patch to avoid shipping the regression in 1.16.0.
This reverts commit b207a932a2.
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=97612
Fixes: https://gitlab.freedesktop.org/cairo/cairo/issues/156
Patch 37a22669 improved performance by using bounding box extents.
However, the code appears to be incorrect. If extents is non-NULL it
copies its contents to group->extents, otherwise it sets group->extents
to sensible defaults, but then goes ahead and tries to copy the
undefined contents. This second copy is unnecessary if extents is
non-NULL and will cause a crash if it is NULL.
Drop the extra copy, guessing it's just a typo.
Coverity ID: #1159559
Signed-off-by: Bryce Harrington <bryce@bryceharrington.org>
Reviewed-By: Uli Schlachter <psychon@znc.in>
_cairo_malloc(0) always returns NULL, but has not been used
consistently. This patch replaces many calls to malloc() with
_cairo_malloc().
Fixes: fdo# 101547
CVE: CVE-2017-9814 Heap buffer overflow at cairo-truetype-subset.c:1299
Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
- each annotation was emitted on every page instead of just the page
that contains the annotation
- the document structure did not correctly link to annotation objects
- fix some annotation related memory leaks
