Bug 101349 - Off-by-one error in FcLangSetIndex()
Summary: Off-by-one error in FcLangSetIndex()
Status: RESOLVED FIXED
Alias: None
Product: fontconfig
Classification: Unclassified
Component: library (show other bugs)
Version: 2.12
Hardware: All All
: medium normal
Assignee: fontconfig-bugs
QA Contact: Behdad Esfahbod
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-08 13:59 UTC by Florent Rougon
Modified: 2017-06-09 07:07 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Commit (as output by 'git format-patch') (2.67 KB, patch)
2017-06-08 13:59 UTC, Florent Rougon
Details | Splinter Review

Description Florent Rougon 2017-06-08 13:59:25 UTC
Created attachment 131802 [details] [review]
Commit (as output by 'git format-patch')

As written at: https://lists.freedesktop.org/archives/fontconfig/2017-June/005941.html

This commit fixes a bug that can be reproduced like this:
  - remove all languages starting with 'a' in fc-lang/Makefile.am (in
    ORTH's definition);
  - rebuild fontconfig with this change (-> new fc-lang/fclang.h);
  - create an FcLangSet 'ls1' that contains at least the first language
    from fcLangCharSets (i.e., the first *remaining* in lexicographic
    order); let's assume it is "ba" for the sake of this description;
  - create an FcLangSet 'ls2' that only contains the language "aa" (any
    language starting with 'a' should work as well);
  - check the return value of FcLangSetContains(ls1, ls2);

The expected return value is FcFalse, however it is FcTrue if you use
the code before this commit.

What happens is that FcLangSetIndex() returns 0, because this is the
index of the first slot after the not-found language "aa" in
fcLangCharSets (since we removed all languages starting with 'a').
However, this index happens to be non-negative, therefore
FcLangSetContainsLang() mistakenly infers that the language "aa" was
found in fcLangCharSets, and thus calls FcLangSetBitGet(ls1, 0), which
returns FcTrue since we've put the first remaining language "ba" in the
'ls1' language set.

The "return -low;" statement previously in FcLangSetIndex() was
inconsistent with the final return statement. "return -(low+1);" fixes
this inconsistency as well as the incorrect behavior described above.
Comment 1 Akira TAGOH 2017-06-09 05:38:26 UTC
Thanks. though I'm not sure if this is really likely to happen. but the fix itself makes sense. merged in git.
Comment 2 Florent Rougon 2017-06-09 07:07:08 UTC
Thank you. It's true that I don't expect the problem to manifest itself in any concrete way (before the fix) unless some rather drastic change were made to fontconfig, or the code in question being extracted and used for some other purpose. That's why my reproducing procedure involved removing a few languages!

However, it was a logical error I found when reading the code, and I would have felt uncomfortable just leaving it there. Thanks for having applied the fix!

Regards


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.