| Message ID | 20241101101345.Bluez.v2.1.Ia122d85386d6c2fc69f5b3d7ea7a7169f73756e4@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Bluez,v2] textfile: Fix possible bad memory access in find_key | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=905230 ---Test result--- Test Summary: CheckPatch PASS 0.44 seconds GitLint PASS 0.30 seconds BuildEll PASS 24.50 seconds BluezMake PASS 1602.43 seconds MakeCheck PASS 13.03 seconds MakeDistcheck PASS 177.50 seconds CheckValgrind PASS 251.72 seconds CheckSmatch PASS 355.53 seconds bluezmakeextell PASS 119.58 seconds IncrementalBuild PASS 1378.91 seconds ScanBuild PASS 984.15 seconds --- Regards, Linux Bluetooth
Dear Howard, Thank you for your patch. Am 01.11.24 um 03:13 schrieb Howard Chung: > From: Yun-Hao Chung <howardchung@google.com> > > If the searched key is a prefix of the first key in the textfile, > the code will assume it's not the first line which is wrong. > > The issue can be reproduced by a fuzzer. > > Stack trace: > #0 0x55e1c450e7ce in find_key /src/bluez/src/textfile.c:133:9 > #1 0x55e1c450e7ce in write_key /src/bluez/src/textfile.c:244:8 > #2 0x55e1c450dc33 in LLVMFuzzerTestOneInput /src/fuzz_textfile.c:61:3 > (...trace in fuzzer) > --- > This is reproduced by https://issues.oss-fuzz.com/issues/42515619 I’d also add the URL to the commit message. Also for the OSS Fuzz ignorant, how would I reproduce the issue? Could you please add the commands? > Changes in v2: > - Add stack trace in commit message > > src/textfile.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/textfile.c b/src/textfile.c > index 313098f38..8188d2ebe 100644 > --- a/src/textfile.c > +++ b/src/textfile.c > @@ -127,10 +127,10 @@ static inline char *find_key(char *map, size_t size, const char *key, size_t len > while (ptrlen > len + 1) { > int cmp = (icase) ? strncasecmp(ptr, key, len) : strncmp(ptr, key, len); > if (cmp == 0) { > - if (ptr == map && *(ptr + len) == ' ') > - return ptr; > - > - if ((*(ptr - 1) == '\r' || *(ptr - 1) == '\n') && > + if (ptr == map) { > + if (*(ptr + len) == ' ') > + return ptr; > + } else if ((*(ptr - 1) == '\r' || *(ptr - 1) == '\n') && > *(ptr + len) == ' ') > return ptr; > } The diff looks good. Kind regards, Paul
Hi Paul, > I’d also add the URL to the commit message. Will do in the next patch > Also for the OSS Fuzz ignorant, how would I reproduce the issue? Could you please add the commands? To reproduce the problem, I simply followed the description in https://google.github.io/oss-fuzz/advanced-topics/reproducing/ Here is the command to build environment for bluez and reproduce the issue python infra/helper.py build_image bluez python infra/helper.py build_fuzzers --sanitizer address --architecture x86_64 bluez python infra/helper.py reproduce bluez fuzz_textfile ${PATH_TO_FUZZ_TESTCASE} On Fri, Nov 1, 2024 at 3:06 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote: > > Dear Howard, > > > Thank you for your patch. > > Am 01.11.24 um 03:13 schrieb Howard Chung: > > From: Yun-Hao Chung <howardchung@google.com> > > > > If the searched key is a prefix of the first key in the textfile, > > the code will assume it's not the first line which is wrong. > > > > The issue can be reproduced by a fuzzer. > > > > Stack trace: > > #0 0x55e1c450e7ce in find_key /src/bluez/src/textfile.c:133:9 > > #1 0x55e1c450e7ce in write_key /src/bluez/src/textfile.c:244:8 > > #2 0x55e1c450dc33 in LLVMFuzzerTestOneInput /src/fuzz_textfile.c:61:3 > > (...trace in fuzzer) > > --- > > This is reproduced by https://issues.oss-fuzz.com/issues/42515619 > > I’d also add the URL to the commit message. > > Also for the OSS Fuzz ignorant, how would I reproduce the issue? Could > you please add the commands? > > > Changes in v2: > > - Add stack trace in commit message > > > > src/textfile.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/src/textfile.c b/src/textfile.c > > index 313098f38..8188d2ebe 100644 > > --- a/src/textfile.c > > +++ b/src/textfile.c > > @@ -127,10 +127,10 @@ static inline char *find_key(char *map, size_t size, const char *key, size_t len > > while (ptrlen > len + 1) { > > int cmp = (icase) ? strncasecmp(ptr, key, len) : strncmp(ptr, key, len); > > if (cmp == 0) { > > - if (ptr == map && *(ptr + len) == ' ') > > - return ptr; > > - > > - if ((*(ptr - 1) == '\r' || *(ptr - 1) == '\n') && > > + if (ptr == map) { > > + if (*(ptr + len) == ' ') > > + return ptr; > > + } else if ((*(ptr - 1) == '\r' || *(ptr - 1) == '\n') && > > *(ptr + len) == ' ') > > return ptr; > > } > > The diff looks good. > > > Kind regards, > > Paul
diff --git a/src/textfile.c b/src/textfile.c index 313098f38..8188d2ebe 100644 --- a/src/textfile.c +++ b/src/textfile.c @@ -127,10 +127,10 @@ static inline char *find_key(char *map, size_t size, const char *key, size_t len while (ptrlen > len + 1) { int cmp = (icase) ? strncasecmp(ptr, key, len) : strncmp(ptr, key, len); if (cmp == 0) { - if (ptr == map && *(ptr + len) == ' ') - return ptr; - - if ((*(ptr - 1) == '\r' || *(ptr - 1) == '\n') && + if (ptr == map) { + if (*(ptr + len) == ' ') + return ptr; + } else if ((*(ptr - 1) == '\r' || *(ptr - 1) == '\n') && *(ptr + len) == ' ') return ptr; }
From: Yun-Hao Chung <howardchung@google.com> If the searched key is a prefix of the first key in the textfile, the code will assume it's not the first line which is wrong. The issue can be reproduced by a fuzzer. Stack trace: #0 0x55e1c450e7ce in find_key /src/bluez/src/textfile.c:133:9 #1 0x55e1c450e7ce in write_key /src/bluez/src/textfile.c:244:8 #2 0x55e1c450dc33 in LLVMFuzzerTestOneInput /src/fuzz_textfile.c:61:3 (...trace in fuzzer) --- This is reproduced by https://issues.oss-fuzz.com/issues/42515619 Changes in v2: - Add stack trace in commit message src/textfile.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)