-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
lfnGetName might not properly null terminate callerstring #67
Comments
I believe the problem is within lfnGetName(), when the filename stored on the FAT volume has a null terminating char. static size_t lfnGetName(DirLfn_t* ldir, char* name, size_t n) {
uint8_t i;
size_t k = 13*((ldir->order & 0X1F) - 1);
for (i = 0; i < 13; i++) {
uint16_t c = lfnGetChar(ldir, i);
if (c == 0 || k >= (n - 1)) {
k = n - 1;
break;
}
name[k++] = c >= 0X7F ? '?' : c;
}
// Terminate with zero byte.
name[k] = '\0';
return k;
} Inside the loop, when lfnGetChar() returns zero, k is set to n - 1, which causes the null terminating zero to be written at the end of the user's buffer for the case of short string. The c == 0 case should probably break from the loop without altering k, so the null terminating zero is written at the correct location. if (c == 0) break; // do not alter k for this case, so the zero termination is written at correct location
if (k >= (n - 1)) {
k = n - 1;
break;
} |
Or maybe k should be checked first? I'm not sure if the initial compute of k could be too large, or if that's a non-issue? |
I need to rethink how getName works. I suspect there is more than one bug. I think I produced a new bug trying to fix another bug. Long file names are stored like this in the directory. So I am backing though the directory file.
|
I haven't noticed any other problem. |
I verified the bug and the following seems to fix it. I would appreciate if you tested any case you have.
|
Yes, confirmed, that works great. I tested here by reverting to the original code and checking that the problem did indeed return (turns out not all boards & programs reproduce the problem - many cases default to buffers pre-filled with zeros), and then I put this into FatFileLFN.cpp and it definitely does solve the problem. |
Thank for the check. I will post a new beta today. The bss zeroed array caught me with a quick fix for another case. I now have a sketch that makes all filename lengths, fills the buffer with '?' before each call and uses a series of buffer lengths to verify truncation. |
Looks like getName() doesn't always properly null terminate the string it writes into the user's buffer.
Here's the result I'm seeing on a simple program to recursively print directories.
The text was updated successfully, but these errors were encountered: