[SSSD] [Patch] Added null checks to authtok module

Simo Sorce simo at redhat.com
Fri Nov 22 14:41:16 UTC 2013


On Fri, 2013-11-22 at 18:42 +0530, Pallavi Jha wrote:
> 
> Hello,
> 
> Patch with required changes is attached with the mail. Please review
> it.
> 
Thank you, comments inline.

> From bfcbe7d16e325dffb807da096a27955c579174e0 Mon Sep 17 00:00:00 2001
> From: Pallavi Jha <pallavikumarijha at gmail.com>
> Date: Sat, 16 Nov 2013 16:23:01 +0530
> Subject: [PATCH] added null checks to authtok module
> 
> ---
>  src/util/authtok.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/authtok.c b/src/util/authtok.c
> index
> 83e6a1c942a0abc136422bf062be47cbc687bb72..2c67c038bfbeebade462b15eef72e7827fb9f7e4 100644
> --- a/src/util/authtok.c
> +++ b/src/util/authtok.c
> @@ -27,11 +27,17 @@ struct sss_auth_token {
>  
>  enum sss_authtok_type sss_authtok_get_type(struct sss_auth_token
> *tok)
>  {
> +    if (!tok) {
> +        return -1;

-1 is not a valid sss_authtok_type, maybe we should just return
SSS_AUTHTOK_TYPE_EMPTY, or you need to add -1 to the enumeration with an
appropriate name SSS_AUTHTOK_TYPE_FAULTY ?

I wonder also if we should just let also some public functions fail,
this should simply never happen ...

> +    }
>      return tok->type;
>  }
>  
>  size_t sss_authtok_get_size(struct sss_auth_token *tok)
>  {
> +    if (!tok) {
> +        return 0;
> +    }
>      switch (tok->type) {
>      case SSS_AUTHTOK_TYPE_PASSWORD:
>      case SSS_AUTHTOK_TYPE_CCFILE:
> @@ -45,12 +51,18 @@ size_t sss_authtok_get_size(struct sss_auth_token
> *tok)
>  
>  uint8_t *sss_authtok_get_data(struct sss_auth_token *tok)
>  {
> +    if (!tok) {
> +        return NULL;
> +    }
>      return tok->data;
>  }
>  
>  errno_t sss_authtok_get_password(struct sss_auth_token *tok,
>                                   const char **pwd, size_t *len)
>  {
> +    if (!tok) {
> +        return EINVAL;
> +    }

Perhaps EFAULT (as in bad address) is a better error message here ?
Not 100% sure.

>      switch (tok->type) {
>      case SSS_AUTHTOK_TYPE_EMPTY:
>          return ENOENT;
> @@ -70,6 +82,9 @@ errno_t sss_authtok_get_password(struct
> sss_auth_token *tok,
>  errno_t sss_authtok_get_ccfile(struct sss_auth_token *tok,
>                                 const char **ccfile, size_t *len)
>  {
> +    if (!tok) {
> +        return EINVAL;
> +    }
>      switch (tok->type) {
>      case SSS_AUTHTOK_TYPE_EMPTY:
>          return ENOENT;
> @@ -91,11 +106,16 @@ static errno_t sss_authtok_set_string(struct
> sss_auth_token *tok,
>                                        const char *context_name,
>                                        const char *str, size_t len)
>  {
> +    if (!tok || !str) {
> +        return EINVAL;
> +    }
> +
>      size_t size;
>  
>      if (len == 0) {
>          len = strlen(str);
> -    } else {
> +    }
> +    else {
>          while (len > 0 && str[len - 1] == '\0') len--;
>      }
>  
> @@ -121,6 +141,9 @@ static errno_t sss_authtok_set_string(struct
> sss_auth_token *tok,
>  
>  void sss_authtok_set_empty(struct sss_auth_token *tok)
>  {
> +    if (!tok) {
> +        return;
> +    }
>      switch (tok->type) {
>      case SSS_AUTHTOK_TYPE_EMPTY:
>          return;
> @@ -174,6 +197,9 @@ errno_t sss_authtok_set(struct sss_auth_token
> *tok,
>  errno_t sss_authtok_copy(struct sss_auth_token *src,
>                           struct sss_auth_token *dst)
>  {
> +    if (!src || !dst) {
> +        return EINVAL;
> +    }
>      sss_authtok_set_empty(dst);
>  
>      if (src->type == SSS_AUTHTOK_TYPE_EMPTY) {
> @@ -205,7 +231,7 @@ struct sss_auth_token *sss_authtok_new(TALLOC_CTX
> *mem_ctx)
>  
>  void sss_authtok_wipe_password(struct sss_auth_token *tok)
>  {
> -    if (tok->type != SSS_AUTHTOK_TYPE_PASSWORD) {
> +    if (!tok || tok->type != SSS_AUTHTOK_TYPE_PASSWORD) {
>          return;
>      }

Aside from the q. of whether EFAULT may be more appropriate in this case
the rest looks sane.

Simo.
  
-- 
Simo Sorce * Red Hat, Inc * New York




More information about the sssd-devel mailing list