Hi people,
Zbigniew has found a problem with "open" which turns out to be a buffer overflow in several FreeDOS kernel functions :-o In short, when you open "textfile.txtgarbage", the garbage is suspiciously silently ignored and "textfile.txt" opens. Looking at the FreeDOS kernel source code, dos_open() uses split_path() to turn the input filename into a directory plus name pair, which indeed only has 8+3 room for the name itself. So it SHOULD FAIL when the filename (without the directory name) does not fit into 8+3 characters. But it fails to fail! It is implemented using ParseDosName() which can optionally treat * and ? separately and checks whether the basename is followed by "." or not. If yes, the extension length is also measured. However, NEITHER the file name length NOR the length of the extension are checked for allowed length! I believe you could tell function 3d to open "textfiletxt" and it would in reality open "textfile.txt" because the 11 character basename overflows into the memory reserved for max 3 bytes of file name extension. What is worse is that you will probably crash DOS and / or apps by passing bad names to findfirst, open, delete, rmdir, rename, mkdir, (getfattr), (setfattr), open/create via alloc_find_free, truename via dir_exists and maybe other calls I have overlooked. Patching ParseDosName() to return an error when name and / or extension are longer than 8 + 3 character should be easy :-) Thanks for patching! I hope somebody who has write access to our kernel sources and a working toolchain is reading this :-) https://sourceforge.net/p/freedos/svn/HEAD/tree/kernel/branches/UNSTABLE/kernel/dosnames.c You want to return DE_FILENOTFND if nFileCnt > FNAME_SIZE or nExtCnt > FEXT_SIZE *before* you reach line 118, as easy as that :-) The HEAD branch seems to be doing this in different ways, via dir_open and ConvertNameSZToName83 called by that: https://sourceforge.net/p/freedos/svn/HEAD/tree/kernel/trunk/kernel/fatdir.c As you see, ConvertNameSZToName83() at the end of the file just *silently* stops copying anything longer than 11 chars. It jumps to the extension whenever it encounters ".", so you could probably tell FreeDOS to open "textfile.tx.doc" and it would actually open "textfile.doc", ignoring the ".tx" part? Also, it is equally vulnerable to the "textfiletxt opens as if you had said textfile.txt" trick, but at least it will not crash anything. It will just treat "textfile.txtgarbage" as if you had said "textfile.txt" in a crash-free way, while it SHOULD probably return an error. But I do not even see how this function CAN tell the caller about errors at all? Thanks for your thoughts on this :-) Regards, Eric _______________________________________________ Freedos-devel mailing list Freedos-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freedos-devel
