mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] possible buffer overflow in crypt() -- musl-1.2.2
@ 2021-11-04 14:53 Terefang Verigorn
  2021-11-04 15:13 ` [musl] " Terefang Verigorn
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Terefang Verigorn @ 2021-11-04 14:53 UTC (permalink / raw)
  To: musl

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [musl] Re: possible buffer overflow in crypt() -- musl-1.2.2
  2021-11-04 14:53 [musl] possible buffer overflow in crypt() -- musl-1.2.2 Terefang Verigorn
@ 2021-11-04 15:13 ` Terefang Verigorn
  2021-11-04 16:13 ` [musl] " Rich Felker
  2021-11-04 16:32 ` Charlotte Delenk
  2 siblings, 0 replies; 4+ messages in thread
From: Terefang Verigorn @ 2021-11-04 15:13 UTC (permalink / raw)
  To: musl

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [musl] possible buffer overflow in crypt() -- musl-1.2.2
  2021-11-04 14:53 [musl] possible buffer overflow in crypt() -- musl-1.2.2 Terefang Verigorn
  2021-11-04 15:13 ` [musl] " Terefang Verigorn
@ 2021-11-04 16:13 ` Rich Felker
  2021-11-04 16:32 ` Charlotte Delenk
  2 siblings, 0 replies; 4+ messages in thread
From: Rich Felker @ 2021-11-04 16:13 UTC (permalink / raw)
  To: Terefang Verigorn; +Cc: musl

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [musl] possible buffer overflow in crypt() -- musl-1.2.2
  2021-11-04 14:53 [musl] possible buffer overflow in crypt() -- musl-1.2.2 Terefang Verigorn
  2021-11-04 15:13 ` [musl] " Terefang Verigorn
  2021-11-04 16:13 ` [musl] " Rich Felker
@ 2021-11-04 16:32 ` Charlotte Delenk
  2 siblings, 0 replies; 4+ messages in thread
From: Charlotte Delenk @ 2021-11-04 16:32 UTC (permalink / raw)
  To: musl

[-- 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 --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-11-04 16:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-04 14:53 [musl] possible buffer overflow in crypt() -- musl-1.2.2 Terefang Verigorn
2021-11-04 15:13 ` [musl] " Terefang Verigorn
2021-11-04 16:13 ` [musl] " Rich Felker
2021-11-04 16:32 ` Charlotte Delenk

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).