[musl] possible buffer overflow in crypt() -- musl-1.2.2

[musl] possible buffer overflow in crypt() -- musl-1.2.2

[Terefang Verigorn]

hello

crypt.h declares
---
struct crypt_data {
   int initialized;
   char __buf[256];
};
---

but crypt.c uses
---
static char buf[128];
return __crypt_r(key, salt, (struct crypt_data *)buf);
---

the buf[128] should be rather buf[sizeof(crypt_data)]

--
Terefang

[musl] Re: possible buffer overflow in crypt() -- musl-1.2.2

[Terefang Verigorn]

proposed patch:

--- crypt.c     2021-01-15 03:26:00.000000000 +0100
+++ crypt.c.fixed       2021-11-04 16:11:25.540969172 +0100
@@ -9,6 +9,6 @@
         * purely to meet the public API requirements of the crypt_r
         * function; the implementation of crypt_r uses the object
         * purely as a char buffer. */
-       static char buf[128];
-       return __crypt_r(key, salt, (struct crypt_data *)buf);
+       static struct crypt_data buf;
+       return __crypt_r(key, salt, (struct crypt_data *)&buf);
 }

On Thu, Nov 4, 2021 at 3:53 PM Terefang Verigorn <terefang@gmail.com> wrote:
>
> hello
>
> crypt.h declares
> ---
> struct crypt_data {
>    int initialized;
>    char __buf[256];
> };
> ---
>
> but crypt.c uses
> ---
> static char buf[128];
> return __crypt_r(key, salt, (struct crypt_data *)buf);
> ---
>
> the buf[128] should be rather buf[sizeof(crypt_data)]
>
> --
> Terefang



-- 
--
Document My Code? Why do you think they call it "code" ?
--
App developers spend too much time debugging errors in production systems
https://betanews.com/2016/11/03/developers-debugging-production-errors/
--
“The Principle of Priority states (a) you must know the difference
between what is urgent and what is important, and (b) you must do
what’s important first.”
Steven Pressfield (born 1943) American writer

Re: [musl] possible buffer overflow in crypt() -- musl-1.2.2

[Rich Felker]

On Thu, Nov 04, 2021 at 03:53:12PM +0100, Terefang Verigorn wrote:
> hello
> 
> crypt.h declares
> ---
> struct crypt_data {
>    int initialized;
>    char __buf[256];
> };
> ---
> 
> but crypt.c uses
> ---
> static char buf[128];
> return __crypt_r(key, salt, (struct crypt_data *)buf);
> ---
> 
> the buf[128] should be rather buf[sizeof(crypt_data)]

Do you have reason to believe it needs more than 128 bytes? The
crypt_data struct has no inherent relation to what's needed; it was
just needed to provide a public ABI for the caller to have enough
(more than enough, for future-proofing or whatever) storage for the
result.

Rich

Re: [musl] possible buffer overflow in crypt() -- musl-1.2.2

[Charlotte Delenk]

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

Hi,

On Thu, Nov 04, 2021 at 03:53:12PM +0100, Terefang Verigorn wrote:
> hello
> 
> crypt.h declares
> ---
> struct crypt_data {
>    int initialized;
>    char __buf[256];
> };
> ---
> 
> but crypt.c uses
> ---
> static char buf[128];
> return __crypt_r(key, salt, (struct crypt_data *)buf);
> ---
> 
> the buf[128] should be rather buf[sizeof(crypt_data)]

It doesn't appear to be a potential buffer overflow issue. According to
the comment in __crypt_r, the crypt_data struct is only used as an
output buffer. The longest output appears to be around 80 bytes long

> 
> --
> Terefang

-- 
Charlotte

https://keybase.io/darkkirb • GPG Key 3CEF5DDA915AECB0 • https://darkkirb.de

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]