From kde-i18n-doc Sat May 07 22:17:23 2016 From: Michael Pyne Date: Sat, 07 May 2016 22:17:23 +0000 To: kde-i18n-doc Subject: Re: Review Request 127834: ki18n: Fix theoretically possible use-after-free in gettext when using st Message-Id: <20160507221723.17376.64034 () mimi ! kde ! org> X-MARC-Message: https://marc.info/?l=kde-i18n-doc&m=146265948226068 --===============9198364755008863855== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On May 7, 2016, 12:26 p.m., Christoph Feck wrote: > > This commit causes compilation failure: > > > > ki18n/src/gettext.h:130:9: error: 'translation_found' was not declared in this scope > > ki18n/src/gettext.h:180:9: error: 'translation_found' was not declared in this scope > > Chusslove Illich wrote: > Moved the declaration now outside of #ifdef block, should work for you. Sorry about that, I did forget to re-compile after removing my 'force VLA support disabled' local hack. Mea culpa! - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127834/#review95259 ----------------------------------------------------------- On May 7, 2016, 4:12 a.m., Michael Pyne wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127834/ > ----------------------------------------------------------- > > (Updated May 7, 2016, 4:12 a.m.) > > > Review request for KDE Frameworks, Localization and Translation (l10n) and Chusslove Illich. > > > Repository: ki18n > > > Description > ------- > > Coverity noted that some of code for message catalog lookup uses some pointer values after they are freed. Even though the use in question is a simple equality comparison against a different (valid) pointer, it is still undefined behavior according to the C (and C++) language specs and is therefore liable to cause miscompilation and who knows what other kinds of problems. > > This code is not normally enabled, normally a code path that supports variable-length arrays is active, which is not susceptible to this bug. > > Since VLAs are not supported even in current C++ versions, making VLA support mandatory is not feasible, so instead I opted to move the pointer comparisons to a point in the code where the comparison is valid, and then use the saved result later. > > I have also reported the bug to GNU Gettext, since upstream still has the error. It is GNU Gettext bug 47847. > > > Diffs > ----- > > src/gettext.h b06fc90 > > Diff: https://git.reviewboard.kde.org/r/127834/diff/ > > > Testing > ------- > > Code compiles and KDE applications still seem to work fine. I also tested with the changed code path forcibly enabled by disabling VLA support, and things still seemed to work. > > There's not a lot of autotests to choose from, but the KLocalizedString test still passed. > > > Thanks, > > Michael Pyne > > --===============9198364755008863855== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 7bit

This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127834/

On May 7th, 2016, 12:26 p.m. UTC, Christoph Feck wrote:

This commit causes compilation failure:

ki18n/src/gettext.h:130:9: error: 'translation_found' was not declared in this scope
ki18n/src/gettext.h:180:9: error: 'translation_found' was not declared in this scope

On May 7th, 2016, 12:39 p.m. UTC, Chusslove Illich wrote:

Moved the declaration now outside of #ifdef block, should work for you.

Sorry about that, I did forget to re-compile after removing my 'force VLA support disabled' local hack. Mea culpa!

- Michael

On May 7th, 2016, 4:12 a.m. UTC, Michael Pyne wrote:

Review request for KDE Frameworks, Localization and Translation (l10n) and Chusslove Illich.

By Michael Pyne.

Updated May 7, 2016, 4:12 a.m.

Repository: ki18n

Description

Coverity noted that some of code for message catalog lookup uses some pointer values after they are freed. Even though the use in question is a simple equality comparison against a different (valid) pointer, it is still undefined behavior according to the C (and C++) language specs and is therefore liable to cause miscompilation and who knows what other kinds of problems.

This code is not normally enabled, normally a code path that supports variable-length arrays is active, which is not susceptible to this bug.

Since VLAs are not supported even in current C++ versions, making VLA support mandatory is not feasible, so instead I opted to move the pointer comparisons to a point in the code where the comparison is valid, and then use the saved result later.

I have also reported the bug to GNU Gettext, since upstream still has the error. It is GNU Gettext bug 47847.

Testing

Code compiles and KDE applications still seem to work fine. I also tested with the changed code path forcibly enabled by disabling VLA support, and things still seemed to work.

There's not a lot of autotests to choose from, but the KLocalizedString test still passed.

Diffs

src/gettext.h (b06fc90)

--===============9198364755008863855==--