Re: mount(8): strlen + malloc + snprintf == asprintf

[Marc Espie]

Are you retarded ?

Go study the source code.

Re: mount(8): strlen + malloc + snprintf == asprintf

[Tom Cosgrove]

>>> Ali H. Fardan  5-Sep-16 09:09 >>>
>
> On 2016-09-05 11:03, Tom Cosgrove wrote:
> :
> > It does allocate the correct buffer size.  It's got all the
> > information it needs to do that with the format string and the
> > parameters.  Then it returns the buffer address via the `ret'
> > argument.
> >
> > If you don't believe us, read the source code and tell us where we
> > are wrong.
> > 
> > Tom
>
> then that patch does weaken security, the buffer can overflow.

asprintf() allocates the buffer, of the size it needs.  It can't overflow.
It makes no change to security.

The patch is fine - you'll notice it's already been committed.

Tom

Re: mount(8): strlen + malloc + snprintf == asprintf

[Martijn van Duren]

On 09/05/16 10:06, Ali H. Fardan wrote:
> On 2016-09-05 11:04, Otto Moerbeek wrote:
>> On Mon, Sep 05, 2016 at 10:47:06AM +0300, Ali H. Fardan wrote:
>>
>>> On 2016-09-05 10:44, David Gwynne wrote:
> On 5 Sep 2016, at 17:39, Ali H. Fardan  wrote:
>
> and why is he telling me this? I just said if the destination is a
> pointer to char, how would a function automagically allocate a size
> for it?

 its not a pointer to a char, its a pointer to a char pointer:

 as per the man page:

  int
  asprintf(char **ret, const char *format, ...);

 dlg

>>>
>>> Still doesn't mean that it can automagically allocate a correct
>>> buffer size.
>>
>> Yes it does.
>>
>> Arguing about this doesn't help anybody. Go study some C.
>>
>>  -Otto
> 
> You got no explanation for your argument.
> 
http://man.openbsd.org/asprintf
/usr/src/lib/libc/stdio/asprintf.c
/usr/src/lib/libc/stdio/vfprintf.c

Come back when you find the bug.
Patches welcome.

Re: mount(8): strlen + malloc + snprintf == asprintf

[Ali H. Fardan]

On 2016-09-05 11:03, Tom Cosgrove wrote:

Ali H. Fardan  5-Sep-16 08:47 >>>


On 2016-09-05 10:44, David Gwynne wrote:
>> On 5 Sep 2016, at 17:39, Ali H. Fardan  wrote:
>>
>> and why is he telling me this? I just said if the destination is a
>> pointer to char, how would a function automagically allocate a size
>> for it?
>
> its not a pointer to a char, its a pointer to a char pointer:
>
> as per the man page:
>
>  int
>  asprintf(char **ret, const char *format, ...);
>
> dlg

Still doesn't mean that it can automagically allocate a correct
buffer size.


It does allocate the correct buffer size.  It's got all the information 
it
needs to do that with the format string and the parameters.  Then it 
returns

the buffer address via the `ret' argument.

If you don't believe us, read the source code and tell us where we are 
wrong.


Tom


then that patch does weaken security, the buffer can overflow.

Re: mount(8): strlen + malloc + snprintf == asprintf

[Tom Cosgrove]

>>> Ali H. Fardan  5-Sep-16 08:47 >>>
>
> On 2016-09-05 10:44, David Gwynne wrote:
> >> On 5 Sep 2016, at 17:39, Ali H. Fardan  wrote:
> >> 
> >> and why is he telling me this? I just said if the destination is a
> >> pointer to char, how would a function automagically allocate a size
> >> for it?
> > 
> > its not a pointer to a char, its a pointer to a char pointer:
> > 
> > as per the man page:
> > 
> >  int
> >  asprintf(char **ret, const char *format, ...);
> > 
> > dlg
>
> Still doesn't mean that it can automagically allocate a correct
> buffer size.

It does allocate the correct buffer size.  It's got all the information it
needs to do that with the format string and the parameters.  Then it returns
the buffer address via the `ret' argument.

If you don't believe us, read the source code and tell us where we are wrong.

Tom

Re: mount(8): strlen + malloc + snprintf == asprintf

[Janne Johansson]

The 3 lines of code it replaced could, so why would you not believe
asprintf() couldn't ?


2016-09-05 9:47 GMT+02:00 Ali H. Fardan :

> On 2016-09-05 10:44, David Gwynne wrote:
>
>> On 5 Sep 2016, at 17:39, Ali H. Fardan  wrote:
>>>
>>> and why is he telling me this? I just said if the destination is a
>>> pointer to char, how would a function automagically allocate a size
>>> for it?
>>>
>>
>> its not a pointer to a char, its a pointer to a char pointer:
>>
>> as per the man page:
>>
>>  int
>>  asprintf(char **ret, const char *format, ...);
>>
>> dlg
>>
>>
> Still doesn't mean that it can automagically allocate a correct
> buffer size.
>
>


-- 
May the most significant bit of your life be positive.

Re: mount(8): strlen + malloc + snprintf == asprintf

[Ali H. Fardan]

On 2016-09-05 11:04, Otto Moerbeek wrote:

On Mon, Sep 05, 2016 at 10:47:06AM +0300, Ali H. Fardan wrote:


On 2016-09-05 10:44, David Gwynne wrote:
> > On 5 Sep 2016, at 17:39, Ali H. Fardan  wrote:
> >
> > and why is he telling me this? I just said if the destination is a
> > pointer to char, how would a function automagically allocate a size
> > for it?
>
> its not a pointer to a char, its a pointer to a char pointer:
>
> as per the man page:
>
>  int
>  asprintf(char **ret, const char *format, ...);
>
> dlg
>

Still doesn't mean that it can automagically allocate a correct
buffer size.


Yes it does.

Arguing about this doesn't help anybody. Go study some C.

-Otto


You got no explanation for your argument.

Re: mount(8): strlen + malloc + snprintf == asprintf

[Otto Moerbeek]

On Mon, Sep 05, 2016 at 10:47:06AM +0300, Ali H. Fardan wrote:

> On 2016-09-05 10:44, David Gwynne wrote:
> > > On 5 Sep 2016, at 17:39, Ali H. Fardan  wrote:
> > > 
> > > and why is he telling me this? I just said if the destination is a
> > > pointer to char, how would a function automagically allocate a size
> > > for it?
> > 
> > its not a pointer to a char, its a pointer to a char pointer:
> > 
> > as per the man page:
> > 
> >  int
> >  asprintf(char **ret, const char *format, ...);
> > 
> > dlg
> > 
> 
> Still doesn't mean that it can automagically allocate a correct
> buffer size.

Yes it does.

Arguing about this doesn't help anybody. Go study some C.

-Otto

Re: mount(8): strlen + malloc + snprintf == asprintf

[Ali H. Fardan]

On 2016-09-05 10:44, David Gwynne wrote:

On 5 Sep 2016, at 17:39, Ali H. Fardan  wrote:

and why is he telling me this? I just said if the destination is a
pointer to char, how would a function automagically allocate a size
for it?


its not a pointer to a char, its a pointer to a char pointer:

as per the man page:

 int
 asprintf(char **ret, const char *format, ...);

dlg



Still doesn't mean that it can automagically allocate a correct
buffer size.

Re: mount(8): strlen + malloc + snprintf == asprintf

[David Gwynne]

> On 5 Sep 2016, at 17:39, Ali H. Fardan <r...@firemail.cc> wrote:
> 
> and why is he telling me this? I just said if the destination is a
> pointer to char, how would a function automagically allocate a size
> for it?

its not a pointer to a char, its a pointer to a char pointer:

as per the man page:

 int
 asprintf(char **ret, const char *format, ...);

dlg

> 
>  Original Message --------
> Subject: Re: mount(8): strlen + malloc + snprintf == asprintf
> Date: 2016-09-05 10:36
> From: "Michael W. Bombardieri" <m...@iinet.net.au>
> To: "Ali H. Fardan" <r...@firemail.cc>, Otto Moerbeek <o...@drijf.net>
> Cc: David Gwynne <da...@gwynne.id.au>, tech <tech@openbsd.org>, 
> owner-t...@openbsd.org
> 
> FWIW the reply seemed like a proper statement to me.
> 
> The manual page for asprintf() doesn't explain its internals. Do you expect 
> someone to give you a summary of asprintf() internals? I don't see why they 
> should.
> 
> On 2016-09-05 3:15 PM, Ali H. Fardan wrote:
>> On 2016-09-05 08:52, Otto Moerbeek wrote:
>>> On Mon, Sep 05, 2016 at 08:05:40AM +0300, Ali H. Fardan wrote:
>>>> On 2016-09-05 08:01, David Gwynne wrote:
>>>> > > On 5 Sep 2016, at 12:13, Ali H. Fardan <r...@firemail.cc> wrote:
>>>> > >
>>>> > > You can't specify a buffer size in asprintf() therefore, it is not
>>>> > > secure,
>>>> > > you can see that snprintf() does write to the `i` bytes to the buffer
>>>> >
>>>> > asprintf allocates the memory it needs to write to, unlike snprintf
>>>> > which requires a preallocated buffer.
>>>> when the destination is a pointer to a char, and the passed argument is a
>>>> memory address, how is it supposed to determine the correct buffer size?
>>>> Raiz
>>> asprintf uses the internals of the printf family of functions. Look in
>>> src/lib/libc/stdio for all the details.
>>>-Otto
>> If you can read my statement and reply with a proper statement,
>> I'd appreciate it.
>> Raiz
> 

Fwd: Re: mount(8): strlen + malloc + snprintf == asprintf

[Ali H. Fardan]

and why is he telling me this? I just said if the destination is a
pointer to char, how would a function automagically allocate a size
for it?

 Original Message 
Subject: Re: mount(8): strlen + malloc + snprintf == asprintf
Date: 2016-09-05 10:36
From: "Michael W. Bombardieri" <m...@iinet.net.au>
To: "Ali H. Fardan" <r...@firemail.cc>, Otto Moerbeek <o...@drijf.net>
Cc: David Gwynne <da...@gwynne.id.au>, tech <tech@openbsd.org>, 
owner-t...@openbsd.org


FWIW the reply seemed like a proper statement to me.

The manual page for asprintf() doesn't explain its internals. Do you 
expect someone to give you a summary of asprintf() internals? I don't 
see why they should.


On 2016-09-05 3:15 PM, Ali H. Fardan wrote:

On 2016-09-05 08:52, Otto Moerbeek wrote:

On Mon, Sep 05, 2016 at 08:05:40AM +0300, Ali H. Fardan wrote:


On 2016-09-05 08:01, David Gwynne wrote:
> > On 5 Sep 2016, at 12:13, Ali H. Fardan <r...@firemail.cc> wrote:
> >
> > You can't specify a buffer size in asprintf() therefore, it is not
> > secure,
> > you can see that snprintf() does write to the `i` bytes to the buffer
>
> asprintf allocates the memory it needs to write to, unlike snprintf
> which requires a preallocated buffer.

when the destination is a pointer to a char, and the passed argument 
is a
memory address, how is it supposed to determine the correct buffer 
size?


Raiz


asprintf uses the internals of the printf family of functions. Look in
src/lib/libc/stdio for all the details.

-Otto


If you can read my statement and reply with a proper statement,
I'd appreciate it.

Raiz

Re: mount(8): strlen + malloc + snprintf == asprintf

[Ted Unangst]

Ali H. Fardan wrote:
> If you can read my statement and reply with a proper statement,
> I'd appreciate it.

You are wrong.

Re: mount(8): strlen + malloc + snprintf == asprintf

[Ali H. Fardan]

On 2016-09-05 08:52, Otto Moerbeek wrote:

On Mon, Sep 05, 2016 at 08:05:40AM +0300, Ali H. Fardan wrote:


On 2016-09-05 08:01, David Gwynne wrote:
> > On 5 Sep 2016, at 12:13, Ali H. Fardan  wrote:
> >
> > You can't specify a buffer size in asprintf() therefore, it is not
> > secure,
> > you can see that snprintf() does write to the `i` bytes to the buffer
>
> asprintf allocates the memory it needs to write to, unlike snprintf
> which requires a preallocated buffer.

when the destination is a pointer to a char, and the passed argument 
is a
memory address, how is it supposed to determine the correct buffer 
size?


Raiz


asprintf uses the internals of the printf family of functions. Look in
src/lib/libc/stdio for all the details.

-Otto


If you can read my statement and reply with a proper statement,
I'd appreciate it.

Raiz

Re: mount(8): strlen + malloc + snprintf == asprintf

[Otto Moerbeek]

On Mon, Sep 05, 2016 at 08:05:40AM +0300, Ali H. Fardan wrote:

> On 2016-09-05 08:01, David Gwynne wrote:
> > > On 5 Sep 2016, at 12:13, Ali H. Fardan  wrote:
> > > 
> > > You can't specify a buffer size in asprintf() therefore, it is not
> > > secure,
> > > you can see that snprintf() does write to the `i` bytes to the buffer
> > 
> > asprintf allocates the memory it needs to write to, unlike snprintf
> > which requires a preallocated buffer.
> 
> when the destination is a pointer to a char, and the passed argument is a
> memory address, how is it supposed to determine the correct buffer size?
> 
> Raiz

asprintf uses the internals of the printf family of functions. Look in
src/lib/libc/stdio for all the details.

-Otto

Re: mount(8): strlen + malloc + snprintf == asprintf

[Ali H. Fardan]

On 2016-09-05 08:01, David Gwynne wrote:

On 5 Sep 2016, at 12:13, Ali H. Fardan  wrote:

You can't specify a buffer size in asprintf() therefore, it is not 
secure,

you can see that snprintf() does write to the `i` bytes to the buffer


asprintf allocates the memory it needs to write to, unlike snprintf
which requires a preallocated buffer.


when the destination is a pointer to a char, and the passed argument is 
a

memory address, how is it supposed to determine the correct buffer size?

Raiz

Re: mount(8): strlen + malloc + snprintf == asprintf

[David Gwynne]

> On 5 Sep 2016, at 12:13, Ali H. Fardan <r...@firemail.cc> wrote:
> 
> You can't specify a buffer size in asprintf() therefore, it is not secure,
> you can see that snprintf() does write to the `i` bytes to the buffer

asprintf allocates the memory it needs to write to, unlike snprintf which 
requires a preallocated buffer.

> 
> Raiz
> 
>  Original Message ----
> Subject: mount(8): strlen + malloc + snprintf == asprintf
> Date: 2016-09-04 19:47
> From: Michal Mazurek <akf...@jasminek.net>
> To: tech@openbsd.org
> 
> do what tb@ did for hexdump
> 
> Index: sbin/mount/mount.c
> ===
> RCS file: /cvs/src/sbin/mount/mount.c,v
> retrieving revision 1.66
> diff -u -p -r1.66 mount.c
> --- sbin/mount/mount.c26 Jun 2016 19:53:40 -  1.66
> +++ sbin/mount/mount.c4 Sep 2016 16:38:41 -
> @@ -685,19 +685,16 @@ maketypelist(char *fslist)
> char *
> catopt(char *s0, const char *s1)
> {
> - size_t i;
>   char *cp;
> 
>   if (s0 && *s0) {
> - i = strlen(s0) + strlen(s1) + 1 + 1;
> - if ((cp = malloc(i)) == NULL)
> + if (asprintf(, "%s,%s", s0, s1) == -1)
>   err(1, NULL);
> - (void)snprintf(cp, i, "%s,%s", s0, s1);
>   } else
>   cp = strdup(s1);
> 
>   free(s0);
> - return (cp);
> + return cp;
> }
> 
> void
> 

Re: mount(8): strlen + malloc + snprintf == asprintf

[Ali H. Fardan]

You can't specify a buffer size in asprintf() therefore, it is not 
secure,

you can see that snprintf() does write to the `i` bytes to the buffer

Raiz

 Original Message 
Subject: mount(8): strlen + malloc + snprintf == asprintf
Date: 2016-09-04 19:47
From: Michal Mazurek <akf...@jasminek.net>
To: tech@openbsd.org

do what tb@ did for hexdump

Index: sbin/mount/mount.c
===
RCS file: /cvs/src/sbin/mount/mount.c,v
retrieving revision 1.66
diff -u -p -r1.66 mount.c
--- sbin/mount/mount.c  26 Jun 2016 19:53:40 -  1.66
+++ sbin/mount/mount.c  4 Sep 2016 16:38:41 -
@@ -685,19 +685,16 @@ maketypelist(char *fslist)
 char *
 catopt(char *s0, const char *s1)
 {
-   size_t i;
char *cp;

if (s0 && *s0) {
-   i = strlen(s0) + strlen(s1) + 1 + 1;
-   if ((cp = malloc(i)) == NULL)
+   if (asprintf(, "%s,%s", s0, s1) == -1)
err(1, NULL);
-   (void)snprintf(cp, i, "%s,%s", s0, s1);
} else
cp = strdup(s1);

free(s0);
-   return (cp);
+   return cp;
 }

 void

mount(8): strlen + malloc + snprintf == asprintf

[Michal Mazurek]

do what tb@ did for hexdump

Index: sbin/mount/mount.c
===
RCS file: /cvs/src/sbin/mount/mount.c,v
retrieving revision 1.66
diff -u -p -r1.66 mount.c
--- sbin/mount/mount.c  26 Jun 2016 19:53:40 -  1.66
+++ sbin/mount/mount.c  4 Sep 2016 16:38:41 -
@@ -685,19 +685,16 @@ maketypelist(char *fslist)
 char *
 catopt(char *s0, const char *s1)
 {
-   size_t i;
char *cp;
 
if (s0 && *s0) {
-   i = strlen(s0) + strlen(s1) + 1 + 1;
-   if ((cp = malloc(i)) == NULL)
+   if (asprintf(, "%s,%s", s0, s1) == -1)
err(1, NULL);
-   (void)snprintf(cp, i, "%s,%s", s0, s1);
} else
cp = strdup(s1);
 
free(s0);
-   return (cp);
+   return cp;
 }
 
 void

-- 
Michal Mazurek