Re: ps buffer overflow - CVE 2023-4016

--000000000000c0ecb806028f2059
Content-Type: text/plain; charset="UTF-8"

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
>

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

<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 mul=
tiplying the size of the -C option by the size of sel_union can result in a=
n overflow 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 xma=
lloc(strlen(arg)*sizeof(sel_union)) to xcalloc(strlen(arg), sizeof(sel_unio=
n)) because calloc specifically calls out and will error on this sort of is=
sue.</div><div>Moving the allocation for node-&gt;u to when we know the num=
ber of 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=
>index 248aa74..15873df 100644<br>--- a/src/ps/parser.c<br>+++ b/src/ps/par=
ser.c<br>@@ -189,7 +189,6 @@ static const char *parse_list(const char *arg,=
 const 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/**=
* prepare to operate ***/<br>=C2=A0 =C2=A0node =3D xmalloc(sizeof(selection=
_node));<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 item=
s ***/<br>@@ -210,6 +209,7 @@ static const char *parse_list(const char *arg=
, const 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=
 items;<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;<b=
r>=C2=A0 =C2=A0while(items--){</div><div><br></div><div>I&#39;ll probably l=
ook 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- Cra=
ig</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 h=
ref=3D"mailto:[email protected]" target=3D"_blank">[email protected]=
.il</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"marg=
in:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1e=
x"><div dir=3D"auto">Hello,<div dir=3D"auto"><br><div dir=3D"auto">We have =
detected this CVE using an automatic static analysis tool we have wrote as =
part of an academic research.</div><div dir=3D"auto"><br></div><div dir=3D"=
auto">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 mis=
understanding regarding reaching you.</div><div dir=3D"auto"><br></div><div=
 dir=3D"auto"><br></div><div dir=3D"auto">Best, Michael</div></div></div>
</blockquote></div>

--000000000000c0ecb806028f2059--