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(a)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