Shouldn't we be fixing our function interfaces to remove inappropriate I32 U32?

I'm thinking we should go through embed.fnc looking for these parameter 
types that are specifying lengths, and change them, even if these are in 
the public API.  Aren't these are segfaults and DOS attacks waiting to 
happen?

Zefram said we did something similar a while back with array indices.

I'm unsure of the implications for modules that can work on earlier perls.
0
public
8/10/2017 10:48:58 PM
perl.perl5.porters 46657 articles. 0 followers. Follow

5 Replies
23 Views

Similar Articles

[PageSpeed] 46

Karl Williamson wrote:
>I'm unsure of the implications for modules that can work on earlier perls.

The implication is that the module may need to define a typedef for
that part of the API, in a Perl-version-dependent manner.  For example,
some of my modules have

    #if PERL_VERSION_GE(5,19,4)
    typedef SSize_t array_ix_t;
    #else /* <5.19.4 */
    typedef I32 array_ix_t;
    #endif /* <5.19.4 */

Whether this is required depends on the details of the module's code.
Not every use of API functions involving the changed type requires the
module to be this aware.  With array indexing, a simple "av_fetch(av, 0,
0)" (with a fixed small index) doesn't require it, but iterating over
a variable-length array requires it, because the module must declare
variables in which to store the top index and the iteration index.

In practice, a lot of preexisting modules probably don't get this kind
of update.  The module will generally still compile, probably without
even a warning, and will still work fine as long as the values seen stay
within the limits of the old type.  They'll go spectacularly wrong if
they actually encounter an object bigger than the old type can represent.

-zefram
0
zefram
8/10/2017 11:18:58 PM
Karl Williamson wrote:
> I'm thinking we should go through embed.fnc looking for these parameter
> types that are specifying lengths, and change them, even if these are in
> the public API.  Aren't these are segfaults and DOS attacks waiting to
> happen?
> 
> Zefram said we did something similar a while back with array indices.
> 
> I'm unsure of the implications for modules that can work on earlier perls.

The main problem to watch out for is I32 pointers.  I32* parameters cannot be changed without really breaking things.

Also, be aware that not every use of I32 is bad.
0
sprout
8/11/2017 1:02:11 AM
--001a11c033d617a6d705568f3528
Content-Type: text/plain; charset="UTF-8"

Father Chrysostomos wrote:
> Date: 11 Aug 2017 01:02:11 -0000
> Subject: Re: Shouldn't we be fixing our function interfaces to remove
inappropriate I32 U32?
> Karl Williamson wrote:
> > I'm thinking we should go through embed.fnc looking for these parameter
> > types that are specifying lengths, and change them, even if these are in
> > the public API.  Aren't these are segfaults and DOS attacks waiting to
> > happen?
> >
> > Zefram said we did something similar a while back with array indices.
> >
> > I'm unsure of the implications for modules that can work on earlier
perls.
>
> The main problem to watch out for is I32 pointers.  I32* parameters
cannot be changed without really breaking things.

> Also, be aware that not every use of I32 is bad.

I  certainly found the I32 typedef confusing in pp_sort.c. It rather shouts
"32-bits" when it is nothing of the (ahem) sort. I'll leave it alone in
what I'm doing, but it certainly can mislead.

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

<div dir=3D"ltr"><span style=3D"font-size:12.8px">Father Chrysostomos wrote=
:</span><br style=3D"font-size:12.8px"><span style=3D"font-size:12.8px">&gt=
; Date:=C2=A011 Aug 2017 01:02:11 -0000</span><br style=3D"font-size:12.8px=
"><span style=3D"font-size:12.8px">&gt; Subject:=C2=A0Re: Shouldn&#39;t we =
be fixing our function interfaces to remove inappropriate I32 U32?</span><b=
r style=3D"font-size:12.8px"><span style=3D"font-size:12.8px">&gt; Karl Wil=
liamson wrote:</span><br style=3D"font-size:12.8px"><span style=3D"font-siz=
e:12.8px">&gt; &gt; I&#39;m thinking we should go through embed.fnc looking=
 for these parameter</span><br style=3D"font-size:12.8px"><span style=3D"fo=
nt-size:12.8px">&gt; &gt; types that are specifying lengths, and change the=
m, even if these are in</span><br style=3D"font-size:12.8px"><span style=3D=
"font-size:12.8px">&gt; &gt; the public API.=C2=A0 Aren&#39;t these are seg=
faults and DOS attacks waiting to</span><br style=3D"font-size:12.8px"><spa=
n style=3D"font-size:12.8px">&gt; &gt; happen?</span><br style=3D"font-size=
:12.8px"><span style=3D"font-size:12.8px">&gt; &gt;</span><br style=3D"font=
-size:12.8px"><span style=3D"font-size:12.8px">&gt; &gt; Zefram said we did=
 something similar a while back with array indices.</span><br style=3D"font=
-size:12.8px"><span style=3D"font-size:12.8px">&gt; &gt;</span><br style=3D=
"font-size:12.8px"><span style=3D"font-size:12.8px">&gt; &gt; I&#39;m unsur=
e of the implications for modules that can work on earlier perls.</span><br=
 style=3D"font-size:12.8px">&gt;<br style=3D"font-size:12.8px"><span style=
=3D"font-size:12.8px">&gt; The main problem to watch out for is I32 pointer=
s.=C2=A0 I32* parameters cannot be changed without really breaking things.<=
/span><br style=3D"font-size:12.8px"><br style=3D"font-size:12.8px"><span s=
tyle=3D"font-size:12.8px">&gt; Also, be aware that not every use of I32 is =
bad.</span><br style=3D"font-size:12.8px"><div><span style=3D"font-size:12.=
8px"><br></span></div><div><span style=3D"font-size:12.8px">I =C2=A0certain=
ly found the I32 typedef confusing in pp_sort.c. It rather shouts &quot;32-=
bits&quot; when it is nothing of the (ahem) sort. I&#39;ll leave it alone i=
n what I&#39;m doing, but it certainly can mislead.</span></div></div>

--001a11c033d617a6d705568f3528--
0
jpl
8/12/2017 2:25:54 PM
--001a114415687ea6c705568ff4c7
Content-Type: text/plain; charset="UTF-8"

On Fri, Aug 11, 2017 at 3:02 AM, Father Chrysostomos <sprout@cpan.org>
wrote:

> Karl Williamson wrote:
> > I'm thinking we should go through embed.fnc looking for these parameter
> > types that are specifying lengths, and change them, even if these are in
> > the public API.  Aren't these are segfaults and DOS attacks waiting to
> > happen?
> >
> > Zefram said we did something similar a while back with array indices.
> >
> > I'm unsure of the implications for modules that can work on earlier
> perls.
>
> The main problem to watch out for is I32 pointers.  I32* parameters cannot
> be changed without really breaking things.
>
> Also, be aware that not every use of I32 is bad.
>

Fortunately, we have very few of those in the API. hv_iterkey is the only
one that looks problematic to me.

Leon

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

<div dir=3D"ltr"><div class=3D"gmail_extra"><div class=3D"gmail_quote">On F=
ri, Aug 11, 2017 at 3:02 AM, Father Chrysostomos <span dir=3D"ltr">&lt;<a h=
ref=3D"mailto:sprout@cpan.org" target=3D"_blank">sprout@cpan.org</a>&gt;</s=
pan> wrote:<br><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0p=
x 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=
=3D"gmail-HOEnZb"><div class=3D"gmail-h5">Karl Williamson wrote:<br>
&gt; I&#39;m thinking we should go through embed.fnc looking for these para=
meter<br>
&gt; types that are specifying lengths, and change them, even if these are =
in<br>
&gt; the public API.=C2=A0 Aren&#39;t these are segfaults and DOS attacks w=
aiting to<br>
&gt; happen?<br>
&gt;<br>
&gt; Zefram said we did something similar a while back with array indices.<=
br>
&gt;<br>
&gt; I&#39;m unsure of the implications for modules that can work on earlie=
r perls.<br>
<br>
</div></div>The main problem to watch out for is I32 pointers.=C2=A0 I32* p=
arameters cannot be changed without really breaking things.<br>
<br>
Also, be aware that not every use of I32 is bad.<br>
</blockquote></div><br></div><div class=3D"gmail_extra">Fortunately, we hav=
e very few of those in the API. hv_iterkey is the only one that looks probl=
ematic to me.<br><br></div><div class=3D"gmail_extra">Leon<br></div></div>

--001a114415687ea6c705568ff4c7--
0
fawaka
8/12/2017 3:19:05 PM
On 08/12/2017 09:19 AM, Leon Timmermans wrote:
> On Fri, Aug 11, 2017 at 3:02 AM, Father Chrysostomos <sprout@cpan.org 
> <mailto:sprout@cpan.org>> wrote:
> 
>     Karl Williamson wrote:
>      > I'm thinking we should go through embed.fnc looking for these
>     parameter
>      > types that are specifying lengths, and change them, even if these
>     are in
>      > the public API.  Aren't these are segfaults and DOS attacks
>     waiting to
>      > happen?
>      >
>      > Zefram said we did something similar a while back with array indices.
>      >
>      > I'm unsure of the implications for modules that can work on
>     earlier perls.
> 
>     The main problem to watch out for is I32 pointers.  I32* parameters
>     cannot be changed without really breaking things.
> 
>     Also, be aware that not every use of I32 is bad.
> 
> 
> Fortunately, we have very few of those in the API. hv_iterkey is the 
> only one that looks problematic to me.
> 
> Leon

It was unclear to me what Leon meant, but he means there are very few 
I32* or U32* parameters (or returns) in the public API.

Here's a complete list of ones I think might be problematical:

Anp	|char*	|delimcpy	|NN char* to|NN const char* toend|NN const char* 
from |NN const char* fromend|int delim|NN I32* retlen
np	|char*	|delimcpy_no_escape|NN char* to|NN const char* toend |NN const 
char* from |NN const char* fromend|int delim |NN I32* retlen
ApdR	|char*	|hv_iterkey	|NN HE* entry|NN I32* retlen
ApdR	|SV*	|hv_iternextsv	|NN HV *hv|NN char **key|NN I32 *retlen
Ap	|I32 *	|markstack_grow
Apd	|void	|sv_pos_u2b	|NULLOK SV *const sv|NN I32 *const offsetp|NULLOK 
I32 *const lenp
Apd	|void	|sv_pos_b2u	|NULLOK SV *const sv|NN I32 *const offsetp
ApdR	|CV*	|find_runcv	|NULLOK U32 *db_seqp
pR	|CV*	|find_runcv_where|U8 cond|IV arg |NULLOK U32 *db_seqp
ApoR	|I32*	|hv_riter_p	|NN HV *hv
0
public
8/12/2017 4:04:07 PM
Reply: