[SSSD] [PATCH] Guard against invalid DP messages
Sumit Bose
sbose at redhat.com
Fri Nov 13 13:27:05 UTC 2015
On Fri, Nov 13, 2015 at 02:08:59PM +0100, Jakub Hrozek wrote:
> On Fri, Nov 13, 2015 at 01:40:16PM +0100, Sumit Bose wrote:
> > > Hi,
> > >
> > > attached are patches that only use sss_utf8_check() and work on RHEL-6
> > > also.
> >
> > The patches look good and fixing the issue as expected. CI passes as
> > well http://sssd-ci.duckdns.org/logs/job/33/16/summary.html (the RHEL6
> > failure is unrelated). ACK
> >
> > I just have a minor comment
> >
> > >
> > > +static const char *safe_be_req_err_msg(const char *msg_in,
> > > + int dp_err_type)
> > > +{
> > > + bool ok;
> > > +
> > > + if (msg_in == NULL) {
> > > + /* No custom error, just use default */
> > > + return dp_err_to_string(dp_err_type);
> > > + }
> > > +
> > > + ok = sss_utf8_check((const uint8_t *) msg_in,
> > > + strlen(msg_in));
> > > + if (!ok) {
> > > + DEBUG(SSSDBG_MINOR_FAILURE,
> > > + "Back end message is invalid, using default\n");
> >
> > Maybe something like
> >
> > DEBUG(SSSDBG_MINOR_FAILURE,
> > "Back end message [%s] contains invalid non-UTF8 character, " \
> > "using default\n", msg_in);
> >
> > would be more specific and would help to better identify the source of the
> > message as well? Similar for sbus_request_valist_check().
> >
> > But to fix the issue at hand and not to delay the release this can of
> > course be added later as well.
>
> No, I think it's important to have good debug messages. I used the one
> you suggested, there are no other changes in the patches.
...
>
> +static int sbus_request_valist_check(va_list va, int first_arg_type)
> +{
> + int ret = EOK;
> +#ifdef HAVE_DBUSBASICVALUE
> + int type;
> + va_list va_check;
> + const DBusBasicValue *value;
> + bool ok;
> +
> + va_copy(va_check, va);
> +
> + type = first_arg_type;
> + while (type != DBUS_TYPE_INVALID) {
> + value = va_arg(va_check, const DBusBasicValue*);
> +
> + if (type == DBUS_TYPE_STRING) {
> + ok = sss_utf8_check((const uint8_t *) value->str,
> + strlen(value->str));
> + if (!ok) {
> + DEBUG(SSSDBG_MINOR_FAILURE,
> + "Back end message [%s] contains invalid non-UTF8 " \
> + "characters");
value->str is missing, additionally sbus_request_return_and_finish() is
used by the responders as well so 'Back end message' should be changed
to 'S-Bus message' or similar.
bye,
Sumit
> + ret = EINVAL;
> + break;
> + }
> + }
> + type = va_arg(va_check, int);
> + }
> +
> + va_end(va_check);
> +#endif /* HAVE_DBUSBASICVALUE */
> + return ret;
> +}
> +
More information about the sssd-devel
mailing list