Bug 64678 (CVE-2005-0014)

From: Martin Schulze <joey@infodrom.org>                                        
To: vendor-sec@lst.de                                                            
Subject: [vendor-sec] CAN-2005-001{3,4}: Multiple problems in ncpfs              
                                                                                d  
Erik Sj?lund discovered two vulnerabilities in programs contained in ncpfs:      
                                                                                 
CAN-2005-0013 Unauthorised file access in ncpfs 2.2.x                            
CAN-2005-0014 Buffer overflow in ncpfs 2.2.5.                                    
                                                                                 
I've also contacted the upstream maintainer.                                     
                                 
                                Erik Sj?lund wrote:                                                              
> On debian stable,                                                              
> as root do the following                                                       
>                                                                                
> # apt-get install ncpfs                                                        
> # touch /tmp/foo                                                               
> # chmod 600 /tmp/foo                                                           
> # ls -l --full-time --time=atime /tmp/foo                                      
> -rw-------    1 root     root            0 Fri Jan 07 10:51:55 2005            
> /tmp/foo                                                                       
>                                                                                
> then as a user do the following                                                
>                                                                                
> $ cd ~                                                                         
> $ mkdir foodir                                                                 
> $ ln -s /tmp/foo .nwclient                                                     
> $ ncpmount foodir                                                              
> ncpmount: Connection to specified server does not exist (0x880F) in            
> find_conn_spec                                                                 
>                                                                                
> Then check if the access time of the file has changed. As root do,             
>                                                                                
> # ls -l --full-time --time=atime /tmp/foo                                      
> -rw-------    1 root     root            0 Fri Jan 07 10:54:45 2005            
> /tmp/foo                                                                       
                                                                                 
I couldn't confirm this in a chroot environment but other factors may            
have mitigated the problem.                                                      
                                                                                 
> The file /tmp/foo was read and thus its file permissions were violated.        
> Now what happens if a user links to an interesting device file instead?        
> ( I didn't test this though )                                                  
>                                                                                
> Instead of a soft symlink the same can be achieved with a hard symlink.        
    (symlink || hardlink)                                                            
                                                                                 
> the relevant code piece is in the function ncp_fopen_nwc() in the              
> file ncpfs-2.2.0.18/lib/ncplib.c                                               
>                                                                                
> The problem is that it doesn't check that the file ownership matches           
> getuid().                                                                      
                                                                                 
This is CAN-2005-0013.                                                           
                                                                                 
This patch should be sufficient:                                                 
                                                                                 
--- ncpfs-2.2.0.18.orig/lib/ncplib.c                                             
+++ ncpfs-2.2.0.18/lib/ncplib.c                                                  
@@ -2210,6 +2210,10 @@                                                           
                *err = errno;                                                    
                return NULL;                                                     
        }                                                                        
+        if (st.st_uid != getuid()) {                                            
+                *err = EACCES;                                                  
+                return NULL;                                                    
+        }                                                                       
        if ((st.st_mode & (S_IRWXO | S_IRWXG)) != 0) {                           
                *err = NCPLIB_INVALID_MODE;                                      
                return NULL;                                                     
                                                                                 
> Then to make it safer: Maybe ncpfs shouldn't allow ~/.nwclient to be a         
> link either. And maybe ncpfs should not trust the HOME variable because        
> it is a setuid program.                                                        
                                                                                 
Yes.                                                                             
                                                                                 
It would also be a good idea to drop privileges, read the file, regain           
privileges to fulfil the task that requires root, and maybe drop them            
again when they are not needed anymore.  (--> Hint for Petr as upstream)         
                                                                                 
I'm not sure disallowing ~/.nw* being a symlink would buy us any                 
security.  (I'm rather sure that it doesn't, but it's always good                
to discuss the possibilities)                                                    
                                                                                 
> Now a look at the situation with ncpfs 2.2.5 ( not in debian stable )          
> ------------------------------------------------                               
>                                                                                
> The source code has been restructured, new file names and new function         
> names but the exploit still exist there.                                       
>                                                                                
> First as root create a file:                                                   
>                                                                                
> # touch /tmp/foo                                                               
> # chmod 600 /tmp/foo                                                           
>                                                                                
> Then as a user                                                                 
>                                                                                
> $ export HOME=/tmp                                                             
> $ ln /tmp/foo $HOME/.nwinfos                                                   
> $ ls -l --full-time --time=atime /tmp/foo                                      
> -rw-------  2 root root 0 2005-01-05 20:13:09.822116800 +0100 /tmp/foo         
> $ ncplogin                                                                     
> failed:Cannot attach to tree e:. Err:Server not found (0x8847) 
 
   > $ ls -l --full-time --time=atime /tmp/foo                                      
> -rw-------  2 root root 0 2005-01-05 20:13:55.147983129 +0100 /tmp/foo         
>                                                                                
> readnwinfosfile() in lib/nwclient.c                                            
> does trust $HOME blindly and opens $HOME/.nwinfos and reads lines with         
> fgets()                                                                        
                                                                                 
This is also CAN-2005-0013.                                                      
                                                                                 
Similar patch:                                                                   
                                                                                 
--- ncpfs-2.2.5.orig/lib/nwclient.c                                              
+++ ncpfs-2.2.5/lib/nwclient.c                                                   
@@ -482,6 +482,10 @@                                                             
                 *err = errno;                                                   
                 return NULL;                                                    
         }                                                                       
+        if (st.st_uid != getuid()) {                                            
+                *err = EACCES;                                                  
+                return NULL;                                                    
+        }                                                                       
         if ((st.st_mode & (S_IRWXO | S_IRWXG)) != 0) {                          
                 *err = NCPLIB_INVALID_MODE;                                     
                 return NULL;                                                    
                                                                                 
> And now another thing present in ncpfs-2.2.5 but not in ncpfs-2.2.0            
> --------------------------------------------------------------------           
>                                                                                
> In the file: ncpfs-2.2.5/sutil/ncplogin.c                                      
>                                                                                
> static int opt_set_volume_after_parsing_all_options(struct                     
> ncp_mount_info* info) {                                                        
> char tmpNWPath[1024];                                                          
> int e;                                                                         
>                                                                                
> /* we DID check in main that -V has been specified !*/                         
> strcpy(tmpNWPath,info->remote_path);                                           
> if (info->root_path) {                                                         
>    strcat(tmpNWPath,"/");                                                      
>    strcat(tmpNWPath,info->root_path);                                          
> }                                                                              
>                                                                                
> It seems root_path comes from the command line with no length check            
> done. Potential buffer overflow. But this function is called some way          
> into the program so I think without having any Novell NetWare servers it       
> is hard to try this buffer overflow out.                                       
                                                                                 
This is CAN-2005-0014.                                                           
                                                                                 
Proposed patch:                                                                  
                                                                                 
--- ncpfs-2.2.5.orig/sutil/ncplogin.c                                            
+++ ncpfs-2.2.5/sutil/ncplogin.c                                                 
@@ -166,10 +166,11 @@                                                            
                int e;                                                           
                                                                                 
                 /* we DID check in main that -V has been specified !*/          
-                strcpy(tmpNWPath,info->remote_path);                            
+                memset(tmpNWPath,0,sizeof(tmpNWPath));                          
 +                strncpy(tmpNWPath,sizeof(tmpNWPath)-1,info->remote_path);       
                 if (info->root_path) {                                          
-                        strcat(tmpNWPath,"/");                                  
-                        strcat(tmpNWPath,info->root_path);                      
+                                                                                
+strncat(tmpNWPath,"/",sizeof(tmpNWPath)-strlen(tmpNWPath)-1);                   
+                                                                                
+strncat(tmpNWPath,info->root_path,sizeof(tmpNWPath)-strlen(tmpNWPath)-1);       
                 }                                                               
                 /* I keep forgeting typing it in uppercase so let's do it 
here  
+*/                                                                              
                 str_upper(tmpNWPath);                                           
                                                                                 
> cheers,                                                                        
> Erik Sj?lund                                                                   
> Adivo Datakonsult AB                                                           
                                                                                 
I'm also pledging for a coordinated disclosure, e.g. in about two                
weeks, depending on what other vendors and Petr (=upstream) think.               
                                                                                 
Regards,                                                                         
                                                                                 
        Joey                                                                     
                                                                                 
--                                                                               
Given enough thrust pigs will fly, but it's not necessarily a good idea.