getpwnam(NULL) get "segmentation fault"
diff mbox

Message ID 55696C2D.5040200@huawei.com
State New
Headers show

Commit Message

shengyong May 30, 2015, 7:52 a.m. UTC
Hi,

When using getpwnam(), we found that if the parameter is NULL, the program
would get a segmentation fault. I checked the glibc code, and found that
parameter passed to function is defined as a macro ADD_PARAMS and its value
is not checked before using.

It seems serval functions under nss directory doing so. But segmentation
fault is not friendly to users. Maybe we could add some parameter-check
code before using theses parameters like the following. But I don't know
if it is reasonable to do that. Any hint is appreciated.

thanks,
Sheng

Comments

Andrew Pinski May 30, 2015, 8 a.m. UTC | #1
On Sat, May 30, 2015 at 3:52 PM, Sheng Yong <shengyong1@huawei.com> wrote:
> Hi,
>
> When using getpwnam(), we found that if the parameter is NULL, the program
> would get a segmentation fault. I checked the glibc code, and found that
> parameter passed to function is defined as a macro ADD_PARAMS and its value
> is not checked before using.

>
> It seems serval functions under nss directory doing so. But segmentation
> fault is not friendly to users. Maybe we could add some parameter-check
> code before using theses parameters like the following. But I don't know
> if it is reasonable to do that. Any hint is appreciated.



I don't see anywhere in POSIX standard where it says this is well
defined.  We have many different places were we don't check for NULL
pointers including and not limited to memcpy, strcmp, strcpy.  So in
my mind, this should fall in the same idea.

Thanks,
Andrew Pinski

>
> thanks,
> Sheng
>
>
> diff --git a/nss/getXXbyYY.c b/nss/getXXbyYY.c
> index 15fecf8..4ea7ee2 100644
> --- a/nss/getXXbyYY.c
> +++ b/nss/getXXbyYY.c
> @@ -96,6 +96,8 @@ FUNCTION_NAME (ADD_PARAMS)
>    /* Get lock.  */
>    __libc_lock_lock (lock);
>
> +  CHECK_PARAMS();
> +
>    if (buffer == NULL)
>      {
>        buffer_size = BUFLEN;
> diff --git a/pwd/getpwnam.c b/pwd/getpwnam.c
> index 9ec66d7..cb28279 100644
> --- a/pwd/getpwnam.c
> +++ b/pwd/getpwnam.c
> @@ -25,5 +25,9 @@
>  #define ADD_PARAMS     const char *name
>  #define ADD_VARIABLES  name
>  #define BUFLEN         NSS_BUFLEN_PASSWD
> +#define CHECK_PARAMS() do {    \
> +  if (name == NULL)            \
> +    return NULL;               \
> +} while (0)
>
>  #include "../nss/getXXbyYY.c"
>

Patch
diff mbox

diff --git a/nss/getXXbyYY.c b/nss/getXXbyYY.c
index 15fecf8..4ea7ee2 100644
--- a/nss/getXXbyYY.c
+++ b/nss/getXXbyYY.c
@@ -96,6 +96,8 @@  FUNCTION_NAME (ADD_PARAMS)
   /* Get lock.  */
   __libc_lock_lock (lock);

+  CHECK_PARAMS();
+
   if (buffer == NULL)
     {
       buffer_size = BUFLEN;
diff --git a/pwd/getpwnam.c b/pwd/getpwnam.c
index 9ec66d7..cb28279 100644
--- a/pwd/getpwnam.c
+++ b/pwd/getpwnam.c
@@ -25,5 +25,9 @@ 
 #define ADD_PARAMS	const char *name
 #define ADD_VARIABLES	name
 #define BUFLEN		NSS_BUFLEN_PASSWD
+#define CHECK_PARAMS()	do {	\
+  if (name == NULL)		\
+    return NULL;		\
+} while (0)

 #include "../nss/getXXbyYY.c"