Bug 444614

Summary: Potential Buffer overflow inside ki18n KCatalog
Product: [Frameworks and Libraries] frameworks-ki18n Reporter: Alvin Wong <alvin>
Component: generalAssignee: Chusslove Illich <caslav.ilic>
Status: REPORTED ---    
Severity: normal CC: amy, kdelibs-bugs
Priority: NOR    
Version: 5.64.0   
Target Milestone: ---   
Platform: Microsoft Windows   
OS: Microsoft Windows   
Latest Commit: Version Fixed In:
Sentry Crash Report:
Attachments: backtrace

Description Alvin Wong 2021-10-29 16:29:05 UTC 
Created attachment 143007 [details]
backtrace

When running Krita with Application Verifier enabled, I get a segfault from within KCatalog::KCatalog [1]. (Note we also apply a patch [2] to it so the line numbers are offset.)

Inspecting `langenv` shows that it is not null-terminated:

(gdb) p langenv
$4 = 0x35522fd0 "LANGUAGE=zh_TW:en_US:zh_TW:en_US:zh_TW:en_"<error: Cannot access memory at address 0x35523000>
(gdb) p langenv[41]
$6 = 95 '_'

However, `langenv` was set using `qsnprintf`, which *should* ensure that the output buffer string is null-terminated, so I don't know what's going on...

[1]: https://invent.kde.org/frameworks/ki18n/-/blob/v5.64.0/src/kcatalog.cpp#L111
[2]: https://invent.kde.org/graphics/krita/-/blob/b6a8e9dfb9d21696bef2ba9523646d954983cf7d/3rdparty/ext_frameworks/0001-ki18n-fix-loading-catalogs-with-patched-gettext.patch
Comment 1 amyspark 2021-10-29 17:42:22 UTC 
https://doc.qt.io/qt-5/qbytearray.html#qvsnprintf

> Warning: Since vsnprintf() shows different behavior on certain platforms, you should not rely on the return value or on the fact that you will always get a 0 terminated string back.

So that's known undefined behaviour. This has been broken from 5.20 onwards: https://invent.kde.org/frameworks/ki18n/-/commit/c2557d6d5ff9f8d4572b537a483740c17be59569

Reassigning to frameworks-ki18n.
Comment 2 Bug Janitor Service 2021-10-30 14:17:37 UTC 
A possibly relevant merge request was started @ https://invent.kde.org/graphics/krita/-/merge_requests/1131
Comment 3 Halla Rempt 2021-11-02 12:37:34 UTC 
Git commit 2face9aa95140b728daa26e67889e33d0d7487e4 by Halla Rempt, on behalf of Alvin Wong.
Committed on 02/11/2021 at 12:37.
Pushed by rempt into branch 'master'.

ext_frameworks: Fix potential buffer overflow on Windows

A  +25   -0    3rdparty/ext_frameworks/0002-ki18n-Ensure-langenv-string-is-null-terminated.patch
M  +1    -0    3rdparty/ext_frameworks/CMakeLists.txt

https://invent.kde.org/graphics/krita/commit/2face9aa95140b728daa26e67889e33d0d7487e4
Comment 4 Alvin Wong 2021-11-02 15:10:46 UTC 
Git commit 1841e77292801acd8f8c6ad9d1d62c9aba467d49 by Alvin Wong.
Committed on 02/11/2021 at 15:10.
Pushed by alvinwong into branch 'krita/5.0'.

ext_frameworks: Fix potential buffer overflow on Windows


(cherry picked from commit 2face9aa95140b728daa26e67889e33d0d7487e4)

A  +25   -0    3rdparty/ext_frameworks/0002-ki18n-Ensure-langenv-string-is-null-terminated.patch
M  +1    -0    3rdparty/ext_frameworks/CMakeLists.txt

https://invent.kde.org/graphics/krita/commit/1841e77292801acd8f8c6ad9d1d62c9aba467d49
Comment 5 Ahmad Samir 2022-09-07 15:30:32 UTC 
Git commit 241e0cfa96b1491721f361f1713b3514c58bde56 by Ahmad Samir.
Committed on 05/09/2022 at 17:12.
Pushed by ahmadsamir into branch 'master'.

KCatalog: make setting LANGUAGE env var more robust

- Use std::snprintf, since its docs state the resulting string will be
  null-terminated; also check the return value from that call to
  print warnings
- Bump the size of the allocated char array to 64

Add rudimentary unittest adapted from kcountrytest.cpp.

M  +7    -3    autotests/CMakeLists.txt
A  +50   -0    autotests/kcatalogtest.cpp     [License: LGPL(v2.0+)]
M  +17   -6    src/i18n/kcatalog.cpp

https://invent.kde.org/frameworks/ki18n/commit/241e0cfa96b1491721f361f1713b3514c58bde56