| Change 583480615 by sungyc@sungyc:fig-export-icing-153-change-459:7032:citc on 2023/11/17 13:59:58 |
| |
| [hunspell][vulnerability fix] Fix heap buffer overflow in AffixMgr::compound_check |
| |
| ## Test plan |
| ``` |
| sso_client -location 'https://clusterfuzz.corp.google.com/testcase-detail/download-testcase?id=5877288434466816' > /tmp/testcase-5877288434466816 && \ |
| blaze --blazerc=/dev/null test -c opt --config=asan-fuzzer --test_strategy=local --test_sharding_strategy=disabled \ |
| --test_env=ENABLE_BLAZE_TEST_FUZZING=1 --test_arg=-runs=100 --test_arg=/tmp/testcase-5877288434466816 \ |
| //third_party/hunspell/fuzzers:suggestions_fuzzer |
| ``` |
| |
| ## Description |
| - Variable `i`, `cmin`, `cmax` are changed in every iteration. |
| - Originally buffer `word` has length `len`. |
| - It is possible that `i >= len` and makes all `word + i` or `word[i]` invalid. This is the heap buffer overflow reason. |
| - So we must check if `i < len` before using `word + i` (or any possibility that accesses `word + i` or later. |
| - Also `len` is changed at L1604, so we should copy it out (as `word_len`) in the beginning of the function. |
| |
| Also double check the [latest hunspell AffixMgr](https://github.com/hunspell/hunspell/blob/master/src/hunspell/affixmgr.cxx#L1570), verifying this change is similar to the current solution. |
| |
| PRESUBMIT=passed |
| BUG=298635289 |
| R=adorokhine,mghiware |
| APPROVED=adorokhine,mghiware |
| REQUIRED_REVIEW=1 |
| DELTA=18 (10 added, 0 deleted, 8 changed) |
| DELTA_BY_EXTENSION=cxx=18 |
| OCL=582895680 |
| FIG_CHANGESET=4ab7f2818e7d826b55a4701f0d97153534ff41fd |
| FIG_WORKSPACE=sungyc/153:icing |
| MARKDOWN=true |
| |
| Affected files ... |
| |
| ... //depot//src/hunspell/affixmgr.cxx#15 edit |
| |
| ==== //depot//src/hunspell/affixmgr.cxx#14 - /google/src/files/583480615/depot//src/hunspell/affixmgr.cxx ==== |
| --- /google/src/files/553210266/depot//src/hunspell/affixmgr.cxx 2023-08-02 14:42:00.000000000 -0400 |
| +++ /google/src/files/583480615/depot//src/hunspell/affixmgr.cxx 2023-11-17 16:59:58.000000000 -0500 |
| @@ -1527,6 +1527,10 @@ |
| short wordnum, short numsyllable, short maxwordnum, short wnum, hentry ** words = NULL, |
| char hu_mov_rule = 0, char is_sug = 0, int * info = NULL) |
| { |
| + // Since len will be modified later, let's copy it out to preserve the |
| + // word's len info. |
| + int word_len = len; |
| + |
| int i; |
| short oldnumsyllable, oldnumsyllable2, oldwordnum, oldwordnum2; |
| struct hentry * rv = NULL; |
| @@ -1586,7 +1590,7 @@ |
| do { // simplified checkcompoundpattern loop |
| |
| if (scpd > 0) { |
| - for (; scpd <= numcheckcpd && (!checkcpdtable[scpd-1].pattern3 || |
| + for (; scpd <= numcheckcpd && (!checkcpdtable[scpd-1].pattern3 || i > word_len || |
| strncmp(word + i, checkcpdtable[scpd-1].pattern3, strlen(checkcpdtable[scpd-1].pattern3)) != 0); scpd++); |
| |
| if (scpd > numcheckcpd) break; // break simplified checkcompoundpattern loop |
| @@ -1865,15 +1869,19 @@ |
| // perhaps second word has prefix or/and suffix |
| sfx = NULL; |
| sfxflag = FLAG_NULL; |
| - rv = (compoundflag && !onlycpdrule) ? affix_check((word+i),strlen(word+i), compoundflag, IN_CPD_END) : NULL; |
| + rv = (compoundflag && !onlycpdrule && i < word_len) ? affix_check((word+i),strlen(word+i), compoundflag, IN_CPD_END) : NULL; |
| if (!rv && compoundend && !onlycpdrule) { |
| sfx = NULL; |
| pfx = NULL; |
| + if (i < word_len) { |
| rv = affix_check((word+i),strlen(word+i), compoundend, IN_CPD_END); |
| } |
| + } |
| |
| if (!rv && numdefcpd && words) { |
| + if (i < word_len) { |
| rv = affix_check((word+i),strlen(word+i), 0, IN_CPD_END); |
| + } |
| if (rv && defcpd_check(&words, wnum + 1, rv, NULL, 1)) { |
| free(rwords); |
| return rv_first; |
| @@ -1886,7 +1894,7 @@ |
| TESTAFF(rv->astr, checkcpdtable[scpd-1].cond2, rv->alen))) rv = NULL; |
| |
| // test CHECKCOMPOUNDPATTERN conditions (forbidden compounds) |
| - if (rv && numcheckcpd && scpd == 0 && cpdpat_check(word, i, rv_first, rv, affixed)) rv = NULL; |
| + if (rv && numcheckcpd && scpd == 0 && i < word_len && cpdpat_check(word, i, rv_first, rv, affixed)) rv = NULL; |
| |
| // check non_compound flag in suffix and prefix |
| if ((rv) && |
| @@ -1918,7 +1926,9 @@ |
| |
| if (langnum == LANG_hu) { |
| // calculate syllable number of the word |
| + if (i < word_len) { |
| numsyllable += get_syllable(word + i, strlen(word + i)); |
| + } |
| |
| // - affix syllable num. |
| // XXX only second suffix (inflections, not derivations) |
| @@ -1979,7 +1989,7 @@ |
| rv = compound_check((st+i),strlen(st+i), wordnum+1, |
| numsyllable, maxwordnum, wnum + 1, words, 0, is_sug, info); |
| |
| - if (rv && numcheckcpd && ((scpd == 0 && cpdpat_check(word, i, rv_first, rv, affixed)) || |
| + if (rv && numcheckcpd && i < word_len && ((scpd == 0 && cpdpat_check(word, i, rv_first, rv, affixed)) || |
| (scpd != 0 && !cpdpat_check(word, i, rv_first, rv, affixed)))) rv = NULL; |
| } else { |
| rv=NULL; |
| @@ -1995,7 +2005,7 @@ |
| } |
| |
| // check first part |
| - if (strncmp(rv->word, word + i, rv->blen) == 0) { |
| + if (i < word_len && strncmp(rv->word, word + i, rv->blen) == 0) { |
| char r = *(st + i + rv->blen); |
| *(st + i + rv->blen) = '\0'; |
| |