Summary:
I had a look at some image loading code in kimageformats and found memory
corruption bugs (there might be more):
- oobwrite4b.xcf: OOB write in kimg_xcf:
By overflowing the "size = 3 * ncolors + 4;" calculation, it's possible to make
size == 3 or size == 0, which then allows 1 or 4 bytes to be overwritten:
https://cgit.kde.org/kimageformats.git/tree/src/imageformats/xcf.cpp?id=3f2552f21b1cdef063c2a93cc95d42a8cf907fcf#n484
The values aren't arbitrary, so AFAICT DoS only.
Fix is to move the sanity check for size below the assignment.
- oobread.tga: OOB read in kimg_tga:
By overflowing the "size = tga.width * tga.height * pixel_size" calculation,
it's possible to cause OOB reads later on as the image data array is too small:
https://cgit.kde.org/kimageformats.git/tree/src/imageformats/tga.cpp?id=3f2552f21b1cdef063c2a93cc95d42a8cf907fcf#n192
Fix is to use a 64bit integer instead.
- oobwrite4b.tga/oobwrite507.tga: OOB write in kimg_tga
If RLE is enabled, any size checks are skipped, so it's possible to write
either 128 repetitions of an arbitrary four byte value (oobwrite4b.tga)
or or 507 arbitrary bytes (oobwrite507.tga) out of bounds.
https://cgit.kde.org/kimageformats.git/tree/src/imageformats/tga.cpp?id=3f2552f21b1cdef063c2a93cc95d42a8cf907fcf#n209
Fix is to check for "num" being negative before reading into the buffer.
Also, bail out early if there is no more data available (reading a 65kx65k px image from 14B data takes ages otherwise)
Test Plan:
Stopped crashing and valgrind don't complain anymore.
TGA preview still works for valid files.
Reviewers: aacid
Reviewed By: aacid
Subscribers: lbeltrame, kde-frameworks-devel
Tags: #frameworks
Differential Revision: https://phabricator.kde.org/D18574
Summary:
The code is even simpler this way.
Found by using heaptrack.
Test Plan: the unittest for rgb still passes.
Reviewers: cfeck
Reviewed By: cfeck
Subscribers: jtamate, kde-frameworks-devel
Tags: #frameworks
Differential Revision: https://phabricator.kde.org/D15890
Summary:
Deny any capabilities when there is no QApp instance.
BUG: 397040
Test Plan:
Untested, as I do not experience the bug on my system and had no time to
invest into trying to.
Reviewers: zccrs, dfaure, pino
Reviewed By: dfaure
Subscribers: kde-frameworks-devel
Tags: #frameworks
Differential Revision: https://phabricator.kde.org/D15405
The GIMP image loader had a limit to 16K x 16K pixels, because this would
already exhaust the 2 GByte address space limit of 32 bit systems.
Remove this limit on 64 bit systems to allow the full 32K x 32K size.
BUG: 391970
Differential Revision: https://phabricator.kde.org/D12557
The JSON file argument is passed to Q_PLUGIN_METADATA, which is a no-code
macro at the C++ level and only used to note information used by moc
for the generated moc file.
So when the content of the JSON file has changed, this will not change
anything in the preprocessed source file itself. It only has an effect on
the content of the moc file generated based on it, which is either included
and thus already triggers a dependecy or generated by automoc and compiled
separately into the target with the needed dependencies.
It is automoc which needs to properly trigger a recreation of the moc
file when checking the sources (and at least in 3.9 & 10 does),
and this is nothing that can be influenced by dependency rules.
kra is the native format for Krita and ora the interchange format
for krita, gimp and mypaint (it's also mypaint's native format).
Both formats are simply zip containers with an embedded png.
REVIEW:126675
decodeRLEData() expects a quint16 as length, but the PSD loader calls it
with a quint32.
We do need quint32 for PSD, otherwise it would overflow for images
bigger than 256x256 pixels (it's the pixel count there, i.e. width x
height).
BUG: 354413
FIXED-IN: 5.19.0
REVIEW: 126684
The change to QLatin1String to QStringLiteral had a very nasty
unintended side effect, causing many (but not all) applications to
crash on exit.
Laurent, please be wary with blanket changes on low level code as
they might break things in unexpected ways.
CCMAIL: montel@kde.org
CCMAIL: tittiatcoke@gmail.com
They were already disabled when building with Qt >= 5.3 in commit
3d45b270ea8341d1516d5863cc49884c2744f2f2 because Qt has better plugins
for those image formats. Now that we depend on Qt 5.3 we can remove
them.
REVIEW: 124636
image formats are loaded via qimage/qimagereader and friends, the user/developer does not choose which ones will be used so giving him a warning about sequential devices not being supported is not going to help anyone, only spam their shell/logs.
REVIEW: 123156
Acked by David Edmundson
By using the same strategy as the SoftImage PIC handler (and sharing
some code with it), we should avoid reading the image data incorrectly
on big-endian architectures.
REVIEW: 122650
BUG: 337918
QtImageFormats 5.3 comes with DDS and JPEG-2000 plugins that support
more options and are generally better than our plugins. The only
advantage our plugins offer is that the Qt DDS plugin does not work on
sequential devices, while ours does. This is outweighed by other
improvements, though, such as supporting more variants.
REVIEW: 119590
Frameworks have a convention of naming uninstalled headers in src/ with
a _p at the end of the name, to make it clear they are not part of the
API. None of the headers in KImageFormats are installed, so it is not
really necessary to follow this convention, but we follow it anyway for
the benefit of both humans and tools (like kapidox).
The "+ 1" was causing an unsigned value to be cast to a signed value,
which was then compared with an unsigned value, causing the warning.
Making the constant unsigned fixes this.