Bug 498380

Summary: Integer overflow in XCF parser
Product: [Frameworks and Libraries] frameworks-kimageformats Reporter: iphydf
Component: generalAssignee: Alex Merry <alex.merry>
Status: REPORTED ---    
Severity: normal CC: aacid, kdelibs-bugs-null, mircomir
Priority: NOR    
Version First Reported In: 6.9.0   
Target Milestone: ---   
Platform: Compiled Sources   
OS: All   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:

Description iphydf 2025-01-08 09:47:03 UTC
Here: https://github.com/KDE/kimageformats/blob/1982557a55fb57ccd07e40194b53693d2b0ab40b/src/imageformats/xcf.cpp#L1316

And also in the other 2 places. `-INT_MIN` is undefined behaviour and will cause a crash on UBSan.
Comment 1 iphydf 2025-01-08 09:53:43 UTC
Reproducer:
```
    const QByteArray data = QByteArray::fromBase64(
        "AWdpbXAgeGNmAAAwAAoAAABbAAAAAzMAAAAAAAAAAAAAAAAAAAAgCHAAAC0AAAAAAAgAAAAAAAAg"
        "8AAAAAAAAAAACAAAAAAgAPAAAAAACgCAAAAAAAAAAAAJ2/AAAAAAAAAAACMAAAAACHAAAAAAAAAA"
        "AAgAAAAAAAAg8AAAAAAAAAAACAAAAAAgXPAAAAAACgCAAAAAAAAAAAAJ2/AAAAAAAAAAACIAAAAI"
        "cAAALQAAAAAACAAAAAAAIAhwAAAtAAAAAAAIAAAAAAAAIPAAAAAAAAAAAAgAAAAAIADwAAAAAAoA"
        "gAAAAAAAAAAACdvwAAAAAAAAAAAjAAAKAIAAAAAAAAAAAAnb8AAAAAAAAAAAIgAAAAhwAAAtAAAA"
        "AAAIAAAAAAAgCHAAAAAAAAAg8AAAAAAAAAAACAAAAAAgAPAAPwAACgCAAAAAAAAAAAAJ2/AAAAAA"
        "AAAAAPAAIABQSApg");

    QImage::fromData(data.mid(1), "XCF");
```

Output:
```
external/kimageformats/src/imageformats/xcf.cpp:1315:73: runtime error: negation of -2147483648 cannot be represented in type 'qint32' (aka 'int'); cast to an unsigned type to negate this value to itself
```
Comment 2 Albert Astals Cid 2025-01-08 16:11:26 UTC
Crashes on ubsan are mostly meaningless and not a problem.

Does it actually crash somewhere else?
Comment 3 iphydf 2025-01-08 16:15:43 UTC
It's undefined behaviour. This prevents us from using ubsan in trap mode as a security measure (yes, crashes are better than remote code execution). The zig cross compiler toolchain has this as a (in my view sensible) default. UB should not happen. The code here intends to create a positive value, but actually creates a negative one. I haven't checked what happens if we let it do that.
Comment 4 Albert Astals Cid 2025-01-08 17:55:16 UTC
Are you shipping code with ubsan enabled? Last I remember the recommendation was not to do that since the instrumentation made your code more vulnerable (can't really find the place that mentioned that, so i may have dreamt it)

Anyhow, patches welcome, but if it doesn't crash in real world, it's not a crash.
Comment 5 iphydf 2025-01-08 18:02:04 UTC
It's recommended not to ship with ubsan + runtime, because the runtime opens more attack vectors. We use the no-runtime version.
Comment 6 Mirco Miranda 2025-01-17 06:19:45 UTC
(In reply to iphydf from comment #0)

> And also in the other 2 places. `-INT_MIN` is undefined behaviour and will
> cause a crash on UBSan.

Have you tried the latest version or just 6.9.0?