>> Chris Wareham <ch...@chriswareham.net> wrote:
> Hi folks,
>
> I've been cleaning up the code in the ToolTalk ISAM library
> (cde/lib/tt/mini_isam), and writing some tests around it. However, I've
> run into a nasty 64 bit pointer issue on LP64 systems that I'm unsure
> how to fix.
>
> The source file cde/lib/tt/mini_isam/iscntl.c defines an iscntl
> function that looks like it was modelled on ioctl(2). It acts as a front
> end to a number of other ISAM functions, and is exposed as an RPC
> function elsewhere in the ToolTalk code. The problem is that it's
> declared to return an int (4 bytes), but in one case it is returning a
> pointer to a function (8 bytes) that will be truncated:
>
> case ISCNTL_FATAL:
> ret = (int)_isfatal_error_set_func(va_arg(pvar, intfunc));
> break;
>
> This call is supposed to return the address of the old error callback
> function.
>
> Any suggestions as to how this could be fixed?
This seems horrible, no wonder that ToolTalk has such a bad security
record. Does this mean that remote client
application is able to set error handler pointer to some arbitrary
set of four bytes? Sounds very dangerous.
I am not sure which functions of iscntl are supposed to be called
remotely.
There seems to be some attempt to convert XDR representation
of integer into the function pointer.
bool_t
xdr_Tt_iscntl_args(XDR *xdrs, _Tt_iscntl_args *objp)
{
if (!xdr_int(xdrs, &objp->isfd)) {
return (FALSE);
}
if (!xdr_int(xdrs, &objp->func)) {
return (FALSE);
}
if (!xdr_array(xdrs, (char **)&objp->arg.arg_val, (u_int
*)&objp->arg.arg_len, ISAPPLMAGICLEN, sizeof(char), (xdrproc_t)xdr_char)) {
return (FALSE);
}
return (TRUE);
}
However, I think that the only server implementation
of this function we have (bin/ttdbserverd/dm_server.C:966)
seems to ignore almost all arguments and always return
" 1" (probably emulating iscntl(isfd, ISCNTL_APPLMAGIC_READ, buf))
call:
_Tt_iscntl_results *
_tt_iscntl_1(_Tt_iscntl_args *args, SVCXPRT * /* transp */)
{
static const char *here = "_tt_iscntl_1()";
static _Tt_iscntl_results res;
if (!_tt_check_stale_isfd(args->isfd)) {
res.result = -1;
res.iserrno = ERPC;
_tt_syslog(errstr, LOG_ERR, "%s: _tt_check_stale_isfd() == 0",
here );
} else {
/*
res.result = iscntl(args->isfd, args->func, args->arg.arg_val);
res.iserrno = iserrno;
res.arg.arg_val = args->arg.arg_val;
res.arg.arg_len = args->arg.arg_len;
if (res.result == -1) {
_tt_syslog(errstr, LOG_ERR, "%s: iscntl(): %d",
here, iserrno);
}
*/
res.iserrno = 0;
res.arg.arg_val = " 1";
res.arg.arg_len = 4;
res.result = 0;
}
return (&res);
}
Interesting, that the code doing the actual call
is commented out.
I would change calls to iscntl(isfd, ISCNTL_FATAL, ...)
to some public form of _isfatal_error_set_func(),
and remove ISCNTL_FATAL facility from iscntl altogether.
//Marcin
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
cdesktopenv-devel mailing list
cdesktopenv-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/cdesktopenv-devel
