blob: 92ce6ea744a761d3c26d9a958c5f654883c28a4f [file] [log] [blame] [edit]
Change 638449400 by sungyc@sungyc:fig-export-hunspell-7504-change-2:7510:citc on 2024/05/29 16:28:31
[hunspell][vulnerability fix] Fix pointer out of bound & use of uninitialized value error
## Test plan
```
sso_client -location 'https://clusterfuzz.corp.google.com/testcase-detail/download-testcase?id=5101182575509504' > /tmp/testcase-5101182575509504 && \
blaze --blazerc=/dev/null test --config=fuzztest-msan \
--test_strategy=local \
--test_sharding_strategy=disabled \
--test_env=FUZZTEST_REPLAY=/tmp/testcase-5101182575509504 \
--test_filter=LLVMFuzzer.TestOneInput \
//third_party/hunspell/fuzzers:dict_fuzzer
```
## Description
- Normally `ap` points to a `'/'` character without escaping by `'\'`.
- We will use it as a separator, so assign `*ap = '\0'`.
- Handle the rest part of the string after this separating position, i.e. `ap + 1`.
- However, when `ap` points to `'\0'` (the end of `ts`), it means there is no remaining part, and `ap + 1` shouldn't be accessed.
- This vulnerability bug is caused by accessing `ap + 1` when `ap` is already pointing at `'\0'`, and therefore accidentally uses uninitialized part of a char array.
- So we just need to add an additional check for `*ap`.
PRESUBMIT=passed
BUG=332580178
R=tjbarron
APPROVED=tjbarron
REQUIRED_REVIEW=1
DELTA=1 (0 added, 0 deleted, 1 changed)
DELTA_BY_EXTENSION=cxx=1
OCL=638419480
FIG_CHANGESET=f465be914d7915ab0ad9635aa1fc79faed571048
FIG_WORKSPACE=sungyc/7504:hunspell
MARKDOWN=true
Affected files ...
... //depot//src/hunspell/hashmgr.cxx#13 edit
==== //depot//src/hunspell/hashmgr.cxx#12 - /google/src/files/638449400/depot//src/hunspell/hashmgr.cxx ====
--- /google/src/files/567444076/depot//src/hunspell/hashmgr.cxx 2023-09-21 18:58:36.000000000 -0400
+++ /google/src/files/638449400/depot//src/hunspell/hashmgr.cxx 2024-05-29 19:28:31.000000000 -0400
@@ -445,7 +445,7 @@
ap = strchr(ap,'/');
}
- if (ap) {
+ if (ap && *ap != '\0') {
*ap = '\0';
if (aliasf) {
int index = atoi(ap + 1);