[SSSD] [PATCH] Guard against invalid DP messages

Sumit Bose sbose at redhat.com
Fri Nov 6 15:59:58 UTC 2015


On Fri, Nov 06, 2015 at 04:26:37PM +0100, Jakub Hrozek wrote:

> From 306000f8b72cbb2822a2ddcd8ce4743018651607 Mon Sep 17 00:00:00 2001
> From: Jakub Hrozek <jhrozek at redhat.com>
> Date: Fri, 6 Nov 2015 15:45:19 +0100
> Subject: [PATCH 2/2] DP: Check callback messages for valid UTF-8
> 
> https://fedorahosted.org/sssd/ticket/2861
> 
> Messages passed from Data Provider to responder must be valid UTF-8
> strings. Because providers might not be completely under our control,
> we need to check if the messages we receive are valid UTF-8 and if they
> are not, use a fallback.
> ---
>  src/providers/data_provider_be.c | 43 +++++++++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 16 deletions(-)
> 
> diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
> index 0cbfb0f5b541a17dc152c00e05a293c5ea0971e0..12b04864255101735b54f9ccc89b703b8e00dd75 100644
> --- a/src/providers/data_provider_be.c
> +++ b/src/providers/data_provider_be.c
> @@ -132,6 +132,26 @@ static const char *dp_err_to_string(int dp_err_type)
>      return "Unknown Error";
>  }
>  
> +static const char *safe_be_req_err_msg(const char *msg_in,
> +                                       int dp_err_type)
> +{
> +    dbus_bool_t ok;
> +
> +    if (msg_in == NULL) {
> +        /* No custom error, just use default */
> +        return dp_err_to_string(dp_err_type);
> +    }
> +
> +    ok = dbus_validate_utf8(msg_in, NULL);
> +    if (!ok) {
> +        DEBUG(SSSDBG_MINOR_FAILURE,
> +              "Back end message is invalid, using default\n");
> +        return dp_err_to_string(dp_err_type);
> +    }
> +
> +    return msg_in;
> +}
> +
>  #define REQ_PHASE_ACCESS 0
>  #define REQ_PHASE_SELINUX 1
>  
> @@ -658,11 +678,7 @@ static void get_subdomains_callback(struct be_req *req,
>           */
>          err_maj = dp_err_type;
>          err_min = errnum;
> -        if (errstr) {
> -            err_msg = errstr;
> -        } else {
> -            err_msg = dp_err_to_string(dp_err_type);
> -        }
> +        err_msg = safe_be_req_err_msg(errstr, dp_err_type);

Wouldn't it be even more safe to do this check in
sbus_request_return_and_finish() and not rely on the callers?

bye,
Sumit


More information about the sssd-devel mailing list