Bug 2610 - smbwrapper/smbsh unsafe string error (and possible fix)
smbwrapper/smbsh unsafe string error (and possible fix)
Status: RESOLVED WONTFIX
Product: Samba 3.0
Classification: Unclassified
Component: Client Tools
3.0.13
Sparc Solaris
: P3 normal
: none
Assigned To: Samba Bugzilla Account
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-04-14 09:58 UTC by Julian Bridle
Modified: 2006-07-05 13:13 UTC (History)
0 users

See Also:


Attachments
Wrong patch file submitted - do not use (438 bytes, patch)
2005-04-14 10:03 UTC, Julian Bridle
no flags Details
Patch for unsafe string copy in smbw.c dirent64_convert() (448 bytes, patch)
2005-04-14 10:48 UTC, Julian Bridle
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Bridle 2005-04-14 09:58:38 UTC
smbsh gives the following runtime error on any access of /smb e.g. 'cd /smb' 
or 'ls /smb'

ld.so.1: ls: fatal: relocation error: file /usr/local/samba/lib/smbwrapper.so: 
symbol __unsafe_string_function_usage_here__: referenced symbol not found

The problem boils down to the use of the pstrcpy() macro in 
smbw.c:dirent64_convert() which detects a mismatch between PSTRING_LEN (1024) 
and the filename in struct dirent64 which is declared as char(1).

wrapped.c:readdir64() which calls the function does allocate some 500 odd bytes 
to the dirent buffer, but this is much smaller than the 1024 limit of pstring.

I suspect that the filename being converted is a Windows filename which I 
believe can only be 255 unicode chars maximum (510), so I changed the code to 
directly use the safe_strcpy() function referencing FSTRING_LEN (256) instead 
which I think is safer.

Sorry but I don't know what form you would prefer unsolicited fixes in, so 
here's a context diff of the fix that works for me.

*** smbw.c.orig Thu Apr  7 14:04:49 2005
--- smbw.c      Thu Apr 14 17:42:39 2005
***************
*** 1495,1501 ****
        d64->d_ino = d->d_ino;
        d64->d_off = d->d_off;
        d64->d_reclen = d->d_reclen;
!       pstrcpy(d64->d_name, d->d_name);
  }
  #endif
  #endif
--- 1495,1501 ----
        d64->d_ino = d->d_ino;
        d64->d_off = d->d_off;
        d64->d_reclen = d->d_reclen;
!       safe_strcpy(d64->d_name, d->d_name, sizeof(fstring)-1);
  }
  #endif
  #endif
Comment 1 Julian Bridle 2005-04-14 10:03:40 UTC
Created attachment 1154 [details]
Wrong patch file submitted - do not use
Comment 2 Julian Bridle 2005-04-14 10:48:47 UTC
Created attachment 1157 [details]
Patch for unsafe string copy in smbw.c dirent64_convert()
Comment 3 Julian Bridle 2005-04-15 03:48:41 UTC
Please ignore the patch in the body of the original bug - it's wrong.  The 
correct patch is in the second attachment.
Comment 4 Gerald (Jerry) Carter 2006-04-20 08:03:33 UTC
severity should be determined by the developers and not the reporter.
Comment 5 Gerald (Jerry) Carter 2006-07-05 13:13:03 UTC
smbwrapper to be removed in 3.0.24.