[Bluez PATCH v2] textfile: Fix possible bad memory access in find_key

[Bluez PATCH v2] textfile: Fix possible bad memory access in find_key

[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

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;
 		}
-- 
2.47.0.163.g1226f6d8fa-goog

RE: [Bluez,v2] textfile: Fix possible bad memory access in find_key

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

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

Re: [Bluez PATCH v2] textfile: Fix possible bad memory access in find_key

[Paul Menzel]

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

Re: [Bluez PATCH v2] textfile: Fix possible bad memory access in find_key

[Yun-hao Chung]

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