samba-tool domain join segfaults (4.16)

[Alexander Bokovoy]

On ti, 24 touko 2022, Michael Tokarev via samba-technical wrote:
> [Cc'ing David Mulder]
> 
> 24.05.2022 16:07, Michael Tokarev via samba-technical wrote:
> > 24.05.2022 15:52, Michael Tokarev via samba-technical wrote:
> > > Hi!
> > > 
> > > I'm facing a segfault in samba-tool when doing domain join, in
> > > source3/utils/py_net.c py_net_join_member(). Here it is (with some
> > > omissions for brevity):
> > > 
> > > static PyObject *py_net_join_member(py_net_Object *self, PyObject *args, PyObject *kwargs)
> > > {
> > >          struct libnet_JoinCtx *r = NULL;
> > >          uint8_t no_dns_updates;
> > > ...
> > 
> > gdb) p &r
> > $1 = (struct libnet_JoinCtx **) 0x7fffffffd7b8
> > (gdb) p &no_dns_updates
> > $2 = (uint8_t *) 0x7fffffffd7b7 ""
> > 
> > >          if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|sssssszpp:Join",
> > >                                           discard_const_p(char *, kwnames),
> > >                                           &r->in.dnshostname,
> > >                                           &r->in.upn,
> > >                                           &r->in.account_ou,
> > >                                           &r->in.os_name,
> > >                                           &r->in.os_version,
> > >                                           &r->in.os_servicepack,
> > >                                           &r->in.machine_password,
> > >                                           &r->in.debug,
> > >                                           &no_dns_updates)) {
> > 
> > It looks like when PyArg_ParseTupleAndKeywords() is setting no_dns_updates,
> > it does more than single byte, affecting the value of r too. Note the
> > addresses of the two - r is right on the next byte after no_dns_updates.
> 
> And according to https://docs.python.org/3/c-api/arg.html , 'p' arg
> of PyArg_ParseTupleAndKeywords() expects an argument of type int, not
> [unsigned] char.
> 
> And indeed, this change:
> 
> diff --git a/source3/utils/py_net.c b/source3/utils/py_net.c
> index 0d774bcb805..c331bf30db4 100644
> --- a/source3/utils/py_net.c
> +++ b/source3/utils/py_net.c
> @@ -71 +71 @@ static PyObject *py_net_join_member(py_net_Object *self, PyObject *args, PyObjec
> -       uint8_t no_dns_updates;
> +       int no_dns_updates;
> 
> fixes the segfault and makes samba-tool domain join work.
> 
> But I wonder about in.debug field, - I'm not sure where this
> struct is defined..

It is defined in the libnet_join's IDL file: source3/librpc/idl/libnet_join.idl

                [in] boolean8 debug,

and boolean8 gets translated to uint8_t in the generated C code.

Since Python API really needs 'int' there, we need to use local int
variable and copy its value to &r->in.<variable>. I wonder if other
places are affected as well.

-- 
/ Alexander Bokovoy