Re: ps buffer overflow - CVE 2023-4016

--0000000000005898c306028f875b
Content-Type: text/plain; charset="UTF-8"

Hi Michael,
  Is there also some URL I can reference for this report? I was going to
use the mail list archive but it's broken and only has my replies.
I'll have a go at summarising it at
https://gitlab.com/procps-ng/procps/-/issues/297

 - Craig


On Thu, 10 Aug 2023 at 20:33, Craig Small <[email protected]> wrote:

> Hi Michael,
>   Very interesting!
>
> In short, if I have read the paper correctly is that multiplying the size
> of the -C option by the size of sel_union can result in an overflow and
> allocating the wrong amount of memory.
>
> I believe the fix would be either (or both):
> Changing the xmalloc(strlen(arg)*sizeof(sel_union)) to
> xcalloc(strlen(arg), sizeof(sel_union)) because calloc specifically calls
> out and will error on this sort of issue.
> Moving the allocation for node->u to when we know the number of items,
> e.g. line 213
>
> Combining this we get:
>
> diff --git a/src/ps/parser.c b/src/ps/parser.c
> index 248aa74..15873df 100644
> --- a/src/ps/parser.c
> +++ b/src/ps/parser.c
> @@ -189,7 +189,6 @@ static const char *parse_list(const char *arg, const
> char *(*parse_fn)(char *, s
>    const char *err;       /* error code that could or did happen */
>    /*** prepare to operate ***/
>    node = xmalloc(sizeof(selection_node));
> -  node->u = xmalloc(strlen(arg)*sizeof(sel_union)); /* waste is
> insignificant */
>    node->n = 0;
>    buf = strdup(arg);
>    /*** sanity check and count items ***/
> @@ -210,6 +209,7 @@ static const char *parse_list(const char *arg, const
> char *(*parse_fn)(char *, s
>    } while (*++walk);
>    if(need_item) goto parse_error;
>    node->n = items;
> +  node->u = xcalloc(items, sizeof(sel_union));
>    /*** actually parse the list ***/
>    walk = buf;
>    while(items--){
>
> I'll probably look at some of the other malloc(x * y) calls too, these are
> called once so we don't need the speed of malloc.
>
>  - Craig
>
>
> On Wed, 9 Aug 2023 at 22:48, Michael Berlin <[email protected]>
> wrote:
>
>> Hello,
>>
>> We have detected this CVE using an automatic static analysis tool we have
>> wrote as part of an academic research.
>>
>> Attached is a document that sums up the CVE.
>>
>> Sorry for the late notice, there have been a misunderstanding regarding
>> reaching you.
>>
>>
>> Best, Michael
>>
>

--0000000000005898c306028f875b
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Hi Michael,</div><div>=C2=A0 Is there also some URL I=
 can reference for this report? I was going to use the mail list archive bu=
t it&#39;s broken and only has my replies.</div><div>I&#39;ll have a go at =
summarising it at <a href=3D"https://gitlab.com/procps-ng/procps/-/issues/2=
97">https://gitlab.com/procps-ng/procps/-/issues/297</a></div><div><br></di=
v><div>=C2=A0- Craig</div><div><br></div></div><br><div class=3D"gmail_quot=
e"><div dir=3D"ltr" class=3D"gmail_attr">On Thu, 10 Aug 2023 at 20:33, Crai=
g Small &lt;<a href=3D"mailto:[email protected]">[email protected]</a>&=
gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0=
px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div =
dir=3D"ltr"><div>Hi Michael,</div><div>=C2=A0 Very interesting!</div><div><=
br></div><div>In short, if I have read the paper correctly is that multiply=
ing the size of the -C option by the size of sel_union can result in an ove=
rflow and allocating the wrong amount of memory.</div><div><br></div><div>I=
 believe the fix would be either (or both):</div><div>Changing the xmalloc(=
strlen(arg)*sizeof(sel_union)) to xcalloc(strlen(arg), sizeof(sel_union)) b=
ecause calloc specifically calls out and will error on this sort of issue.<=
/div><div>Moving the allocation for node-&gt;u to when we know the number o=
f items, e.g. line 213</div><div><br></div><div>Combining this we get:</div=
><div><br></div><div>diff --git a/src/ps/parser.c b/src/ps/parser.c<br>inde=
x 248aa74..15873df 100644<br>--- a/src/ps/parser.c<br>+++ b/src/ps/parser.c=
<br>@@ -189,7 +189,6 @@ static const char *parse_list(const char *arg, cons=
t char *(*parse_fn)(char *, s<br>=C2=A0 =C2=A0const char *err; =C2=A0 =C2=
=A0 =C2=A0 /* error code that could or did happen */<br>=C2=A0 =C2=A0/*** p=
repare to operate ***/<br>=C2=A0 =C2=A0node =3D xmalloc(sizeof(selection_no=
de));<br>- =C2=A0node-&gt;u =3D xmalloc(strlen(arg)*sizeof(sel_union)); /* =
waste is insignificant */<br>=C2=A0 =C2=A0node-&gt;n =3D 0;<br>=C2=A0 =C2=
=A0buf =3D strdup(arg);<br>=C2=A0 =C2=A0/*** sanity check and count items *=
**/<br>@@ -210,6 +209,7 @@ static const char *parse_list(const char *arg, c=
onst char *(*parse_fn)(char *, s<br>=C2=A0 =C2=A0} while (*++walk);<br>=C2=
=A0 =C2=A0if(need_item) goto parse_error;<br>=C2=A0 =C2=A0node-&gt;n =3D it=
ems;<br>+ =C2=A0node-&gt;u =3D xcalloc(items, sizeof(sel_union));<br>=C2=A0=
 =C2=A0/*** actually parse the list ***/<br>=C2=A0 =C2=A0walk =3D buf;<br>=
=C2=A0 =C2=A0while(items--){</div><div><br></div><div>I&#39;ll probably loo=
k at some of the other malloc(x * y) calls too, these are called once so we=
 don&#39;t need the speed of malloc.</div><div><br></div><div>=C2=A0- Craig=
</div><div><br></div></div><br><div class=3D"gmail_quote"><div dir=3D"ltr" =
class=3D"gmail_attr">On Wed, 9 Aug 2023 at 22:48, Michael Berlin &lt;<a hre=
f=3D"mailto:[email protected]" target=3D"_blank">[email protected]=
l</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin=
:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"=
><div dir=3D"auto">Hello,<div dir=3D"auto"><br><div dir=3D"auto">We have de=
tected this CVE using an automatic static analysis tool we have wrote as pa=
rt of an academic research.</div><div dir=3D"auto"><br></div><div dir=3D"au=
to">Attached is a document that sums up the CVE.</div><div dir=3D"auto"><br=
></div><div dir=3D"auto">Sorry for the late notice, there have been a misun=
derstanding regarding reaching you.</div><div dir=3D"auto"><br></div><div d=
ir=3D"auto"><br></div><div dir=3D"auto">Best, Michael</div></div></div>
</blockquote></div>
</blockquote></div>

--0000000000005898c306028f875b--