We've been using vfs_expand_msdfs for years with custom modifications. We recently updated from 3.0.x to 3.4.x and found the vfs_expand_msdfs didn't work. This is the error:

[2010/01/28 13:53:58,  0, pid=31373] lib/util_str.c:532(safe_strcpy_fn)
  ERROR: string overflow by 1 (6 - 5) in safe_strcpy [msdfs:servername.washington.edu\homes]

This appears to be the same as bug 5998 reported a long while ago against Samba 3.2. I'd guess that expand_msdfs doesn't get much use since this bug has been around without much complaint.

The bug is revealed because the function msdfs.c:is_msdfs_link_internal() provides a buffer of only 7 bytes to the VFS readlink() call in the case where it is simply trying to test if the link is an msdfs link. Given this 7 byte buf, the function vfs_expand_msdfs.c:expand_msdfs_readlink() does not comply with readlink() semantics. Notably, at the end of the function, it tries to put a string of possibly MAX_PATH length into a buffer that is too small, typically 7 bytes. The safe_strcpy call is what generates the error.

The readlink() semantics are:
1. places the contents of the symbolic link in buf which has bufsiz.
2. does not append a null to buf
3. will truncate the contents to bufsiz characters if buf is too small.
4. returns the number of bytes placed in buf.

In expand_msdfs_readlink(), safe_strcpy() is used to fill the buffer which isn't ideal, especially where the buffer is too small since strcpy requires the dest to be big enough and assumes it should null terminate. This is where the error message occurs. This shouldn't be done with strcpy and buf should be filled with as many bytes as possible and no nul.

Second, readlink() returns the number of bytes placed in the buffer. However, this function tries to do that with strlen() which would fail with a non-null terminated buf or one that was too small to hold the whole target.

I'm a rusty C programmer. Here is a fix that probably could be more elegant but makes expand_msdfs_readlink() comply with readlink() semantics and fixes the problem. Replace this:

        safe_strcpy(buf, target, bufsiz-1);
        return strlen(buf);

With this:

        result = (strlen(target) < bufsiz) ? strlen(target) : bufsiz;
        memcpy(buf, target, result);
        return result;

This will return the smaller of bufsiz or the length of the symlink target. It will fill buf with the full symlink target or truncate it to bufsiz. No nul termination, I might need that byte if my link target is MAX_PATH long.

-Eric