2022-04-25 07:33:09

by Mikulas Patocka

Subject: [PATCH] hex2bin: fix access beyond string end

If we pass too short string to "hex2bin" (and the string size without the
terminating NUL character is even), "hex2bin" reads one byte after the
terminating NUL character. This patch fixes it.

Signed-off-by: Mikulas Patocka <[email protected]>
Cc: [email protected]

---
lib/hexdump.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

Index: linux-2.6/lib/hexdump.c
===================================================================
--- linux-2.6.orig/lib/hexdump.c 2022-04-24 18:51:16.000000000 +0200
+++ linux-2.6/lib/hexdump.c 2022-04-24 18:51:16.000000000 +0200
@@ -45,10 +45,13 @@ EXPORT_SYMBOL(hex_to_bin);
int hex2bin(u8 *dst, const char *src, size_t count)
{
while (count--) {
- int hi = hex_to_bin(*src++);
- int lo = hex_to_bin(*src++);
+ int hi, lo;

- if ((hi < 0) || (lo < 0))
+ hi = hex_to_bin(*src++);
+ if (hi < 0)
+ return -EINVAL;
+ lo = hex_to_bin(*src++);
+ if (lo < 0)
return -EINVAL;

*dst++ = (hi << 4) | lo;

2022-04-27 10:08:48

by Andy Shevchenko

Subject: Re: [PATCH] hex2bin: fix access beyond string end

On Sun, Apr 24, 2022 at 10:48 PM Mikulas Patocka <[email protected]> wrote:
>
> If we pass too short string to "hex2bin" (and the string size without the
> terminating NUL character is even), "hex2bin" reads one byte after the
> terminating NUL character. This patch fixes it.
>
> Signed-off-by: Mikulas Patocka <[email protected]>
> Cc: [email protected]

You need to provide a Fixes tag.

...

> while (count--) {
> - int hi = hex_to_bin(*src++);
> - int lo = hex_to_bin(*src++);
> + int hi, lo;
>
> - if ((hi < 0) || (lo < 0))
> + hi = hex_to_bin(*src++);
> + if (hi < 0)
> + return -EINVAL;

return hi;

> + lo = hex_to_bin(*src++);
> + if (lo < 0)
> return -EINVAL;

return lo;

> *dst++ = (hi << 4) | lo;

And on top of that it would be nice to understand if we need to
support half-bytes, but in any case it's not a scope of the patch
right now.

--
With Best Regards,
Andy Shevchenko