[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