lists.arthurdejong.org
RSS feed

Re: [PATCH] Use an explicit base of 10 for strtouid()/strtogid() calls

[Date Prev][Date Next] [Thread Prev][Thread Next]

Re: [PATCH] Use an explicit base of 10 for strtouid()/strtogid() calls



On Tue, Sep 27, 2011 at 09:53:05PM +0200, Arthur de Jong wrote:
> On Tue, 2011-09-27 at 14:14 +0200, Jakub Hrozek wrote:
> > This patch came up as a suggestion during Nalin's peer code review of my
> > earlier strto* patches.
> > 
> > If a broken LDAP server entry had the uidNumber so it looks like a number
> > from a different base then 10 to strto* functions, we would have converted
> > it in the respective base instead of 10. For instance "010" would have
> > been converted to "8".
> > 
> > I managed to break the configuration by putting "0100" into a custom
> > attribute and then mapping the uidNumber to it by using "map uidNumber
> > foobar". "getent passwd octaluid" then reported 512 for UID.
> 
> It is debatable how 0100 should be interpreted. Also, a value of 0x400
> is currently allowed and would no longer be. I don't think (hope) that
> anyone uses something like this though.

Yeah, I think a hexa value would be very..creative..and it would be better
to just error out. The octal case is far more dangerous b/c you can get
different values than intended by accidentally prepending the uidNumber
value with a 0, the hexa value would most probably not be accidental.

> 
> I'm fine with the change but be sure to also check the other places
> where strto*() is used (shadow/rpc/protocol/service).

Thanks for pointing that out. I only checked the strtouid/strtogid calls
which was wrong.

A new patch is attached.
>From 4c5b0b158f442458cc70172985dad04565ed539a Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Tue, 27 Sep 2011 11:38:49 +0200
Subject: [PATCH] Use an explicit base of 10 for
 strtouid()/strtogid()/strtol() calls

---
 nslcd/cfg.c      |    4 ++--
 nslcd/group.c    |    2 +-
 nslcd/passwd.c   |    6 +++---
 nslcd/protocol.c |    2 +-
 nslcd/rpc.c      |    2 +-
 nslcd/service.c  |    2 +-
 nslcd/shadow.c   |    6 +++---
 7 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/nslcd/cfg.c b/nslcd/cfg.c
index 00dd8c4..e5c3b1a 100644
--- a/nslcd/cfg.c
+++ b/nslcd/cfg.c
@@ -431,7 +431,7 @@ static void get_uid(const char *filename,int lnr,
   
check_argumentcount(filename,lnr,keyword,get_token(line,token,sizeof(token))!=NULL);
   /* check if it is a valid numerical uid */
   errno=0;
-  *var=strtouid(token,&tmp,0);
+  *var=strtouid(token,&tmp,10);
   if ((*token!='\0')&&(*tmp=='\0')&&(errno==0))
     return;
   /* find by name */
@@ -457,7 +457,7 @@ static void get_gid(const char *filename,int lnr,
   
check_argumentcount(filename,lnr,keyword,get_token(line,token,sizeof(token))!=NULL);
   /* check if it is a valid numerical gid */
   errno=0;
-  *var=strtogid(token,&tmp,0);
+  *var=strtogid(token,&tmp,10);
   if ((*token!='\0')&&(*tmp=='\0')&&(errno==0))
     return;
   /* find by name */
diff --git a/nslcd/group.c b/nslcd/group.c
index 41d8f8f..4725295 100644
--- a/nslcd/group.c
+++ b/nslcd/group.c
@@ -281,7 +281,7 @@ static int write_group(TFILE *fp,MYLDAP_ENTRY *entry,const 
char *reqname,
       else
       {
         errno=0;
-        gids[numgids]=strtogid(gidvalues[numgids],&tmp,0);
+        gids[numgids]=strtogid(gidvalues[numgids],&tmp,10);
         if ((*(gidvalues[numgids])=='\0')||(*tmp!='\0'))
         {
           log_log(LOG_WARNING,"%s: %s: non-numeric",
diff --git a/nslcd/passwd.c b/nslcd/passwd.c
index ab097c6..9eba10c 100644
--- a/nslcd/passwd.c
+++ b/nslcd/passwd.c
@@ -195,7 +195,7 @@ static int entry_has_valid_uid(MYLDAP_ENTRY *entry)
     else
     {
       errno=0;
-      uid=strtouid(values[i],&tmp,0);
+      uid=strtouid(values[i],&tmp,10);
       if ((*(values[i])=='\0')||(*tmp!='\0'))
       {
         log_log(LOG_WARNING,"%s: %s: non-numeric",
@@ -492,7 +492,7 @@ static int write_passwd(TFILE *fp,MYLDAP_ENTRY *entry,const 
char *requser,
       else
       {
         errno=0;
-        uids[numuids]=strtouid(tmpvalues[numuids],&tmp,0);
+        uids[numuids]=strtouid(tmpvalues[numuids],&tmp,10);
         if ((*(tmpvalues[numuids])=='\0')||(*tmp!='\0'))
         {
           log_log(LOG_WARNING,"%s: %s: non-numeric",
@@ -530,7 +530,7 @@ static int write_passwd(TFILE *fp,MYLDAP_ENTRY *entry,const 
char *requser,
       return 0;
     }
     errno=0;
-    gid=strtogid(gidbuf,&tmp,0);
+    gid=strtogid(gidbuf,&tmp,10);
     if ((gidbuf[0]=='\0')||(*tmp!='\0'))
     {
       log_log(LOG_WARNING,"%s: %s: non-numeric",
diff --git a/nslcd/protocol.c b/nslcd/protocol.c
index 45d1f9e..90327b8 100644
--- a/nslcd/protocol.c
+++ b/nslcd/protocol.c
@@ -144,7 +144,7 @@ static int write_protocol(TFILE *fp,MYLDAP_ENTRY 
*entry,const char *reqname)
                         myldap_get_dn(entry),attmap_protocol_ipProtocolNumber);
   }
   errno=0;
-  proto=(int)strtol(protos[0],&tmp,0);
+  proto=(int)strtol(protos[0],&tmp,10);
   if ((*(protos[0])=='\0')||(*tmp!='\0'))
   {
     log_log(LOG_WARNING,"%s: %s: non-numeric",
diff --git a/nslcd/rpc.c b/nslcd/rpc.c
index 68d0f9b..90cb89e 100644
--- a/nslcd/rpc.c
+++ b/nslcd/rpc.c
@@ -145,7 +145,7 @@ static int write_rpc(TFILE *fp,MYLDAP_ENTRY *entry,const 
char *reqname)
                         myldap_get_dn(entry),attmap_rpc_oncRpcNumber);
   }
   errno=0;
-  number=(int)strtol(numbers[0],&tmp,0);
+  number=(int)strtol(numbers[0],&tmp,10);
   if ((*(numbers[0])=='\0')||(*tmp!='\0'))
   {
     log_log(LOG_WARNING,"%s: %s: non-numeric",
diff --git a/nslcd/service.c b/nslcd/service.c
index f8a08bc..d0db52d 100644
--- a/nslcd/service.c
+++ b/nslcd/service.c
@@ -173,7 +173,7 @@ static int write_service(TFILE *fp,MYLDAP_ENTRY *entry,
                         myldap_get_dn(entry),attmap_service_ipServicePort);
   }
   errno=0;
-  port=(int)strtol(ports[0],&tmp,0);
+  port=(int)strtol(ports[0],&tmp,10);
   if ((*(ports[0])=='\0')||(*tmp!='\0'))
   {
     log_log(LOG_WARNING,"%s: %s: non-numeric value",
diff --git a/nslcd/shadow.c b/nslcd/shadow.c
index df7f18b..eb5ffcb 100644
--- a/nslcd/shadow.c
+++ b/nslcd/shadow.c
@@ -130,7 +130,7 @@ static long to_date(const char *dn,const char *date,const 
char *attr)
     strncpy(buffer,date,l);
     buffer[l]='\0';
     errno=0;
-    value=strtol(date,&tmp,0);
+    value=strtol(date,&tmp,10);
     if ((*date=='\0')||(*tmp!='\0'))
     {
       log_log(LOG_WARNING,"%s: %s: non-numeric",dn,attr);
@@ -146,7 +146,7 @@ static long to_date(const char *dn,const char *date,const 
char *attr)
        and some value that needs to be added */
   }
   errno=0;
-  value=strtol(date,&tmp,0);
+  value=strtol(date,&tmp,10);
   if ((*date=='\0')||(*tmp!='\0'))
   {
     log_log(LOG_WARNING,"%s: %s: non-numeric",dn,attr);
@@ -169,7 +169,7 @@ static long to_date(const char *dn,const char *date,const 
char *attr)
   if (tmpvalue==NULL) \
     tmpvalue=""; \
   errno=0; \
-  var=strtol(tmpvalue,&tmp,0); \
+  var=strtol(tmpvalue,&tmp,10); \
   if ((*(tmpvalue)=='\0')||(*tmp!='\0')) \
   { \
     log_log(LOG_WARNING,"%s: %s: non-numeric", \
-- 
1.7.6.2

-- 
To unsubscribe send an email to
nss-pam-ldapd-users-unsubscribe@lists.arthurdejong.org or see
http://lists.arthurdejong.org/nss-pam-ldapd-users/