Re: [ntfs-3g-devel] [PATCH] unistr.c: make utf16_to_utf8_size() always honor @outs_len
Hi Jean-Pierre, Are you going to be reviewing/applying any of these other patches? (Excluding the "ACE validation fixes" one which will need to be reworked once the desired behavior is agreed on.) I've also found a bug in lowntfs-3g regarding the reparse plugin support, so I'll send a patch for that too. Thanks, Eric On Wed, Sep 14, 2016 at 11:39:07PM -0700, Eric Biggers wrote: > utf16_to_utf8_size() was not guaranteed to fail with ENAMETOOLONG if the > computed length was greater than @outs_len. This could cause a buffer > overrun in ntfs_utf16_to_utf8(). This was a bug introduced by the > patches to allow broken Unicode. Fix it. > > Signed-off-by: Eric Biggers> --- > libntfs-3g/unistr.c | 26 +- > 1 file changed, 17 insertions(+), 9 deletions(-) > > diff --git a/libntfs-3g/unistr.c b/libntfs-3g/unistr.c > index 4d33bb4..190dbd8 100644 > --- a/libntfs-3g/unistr.c > +++ b/libntfs-3g/unistr.c > @@ -458,10 +458,15 @@ void ntfs_file_value_upcase(FILE_NAME_ATTR > *file_name_attr, > */ > > /* > - * Return the amount of 8-bit elements in UTF-8 needed (without the > terminating > - * null) to store a given UTF-16LE string. > + * Return the number of bytes in UTF-8 needed (without the terminating null) > to > + * store the given UTF-16LE string. > * > - * Return -1 with errno set if string has invalid byte sequence or too long. > + * On error, -1 is returned, and errno is set to the error code. The > following > + * error codes can be expected: > + * EILSEQ The input string is not valid UTF-16LE (only possible > + * if compiled without ALLOW_BROKEN_UNICODE). > + * ENAMETOOLONGThe length of the UTF-8 string in bytes (without the > + * terminating null) would exceed @outs_len. > */ > static int utf16_to_utf8_size(const ntfschar *ins, const int ins_len, int > outs_len) > { > @@ -470,7 +475,7 @@ static int utf16_to_utf8_size(const ntfschar *ins, const > int ins_len, int outs_l > BOOL surrog; > > surrog = FALSE; > - for (i = 0; i < ins_len && ins[i]; i++) { > + for (i = 0; i < ins_len && ins[i] && count <= outs_len; i++) { > unsigned short c = le16_to_cpu(ins[i]); > if (surrog) { > if ((c >= 0xdc00) && (c < 0xe000)) { > @@ -511,17 +516,20 @@ static int utf16_to_utf8_size(const ntfschar *ins, > const int ins_len, int outs_l > count += 3; > else > goto fail; > - if (count > outs_len) { > - errno = ENAMETOOLONG; > - goto out; > - } > } > - if (surrog) > + > + if (surrog && count <= outs_len) { > #if ALLOW_BROKEN_UNICODE > count += 3; /* ending with a single surrogate */ > #else > goto fail; > #endif /* ALLOW_BROKEN_UNICODE */ > + } > + > + if (count > outs_len) { > + errno = ENAMETOOLONG; > + goto out; > + } > > ret = count; > out: > -- > 2.9.3 > -- The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik ___ ntfs-3g-devel mailing list ntfs-3g-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ntfs-3g-devel
Re: [ntfs-3g-devel] [PATCH 2/2] ACE validation fixes
Hi Jean-Pierre, Sorry for the late response. On Mon, Sep 26, 2016 at 01:52:36PM +0200, Jean-Pierre André wrote: > > 1. "Object" ACEs are mentioned as only being used for Active Directory > > objects > > [source: Windows Internals 6th edition]. On Windows, trying to use > > SetFileSecurity() to set an object ACE in the DACL of an NTFS directory > > fails > > with ERROR_INVALID_ACL. This is different from how Windows treats truly > > unknown ACE types (see below). But I think it would be fine for > > NTFS-3G to > > simplify things by treating object ACEs like any other unknown ACE type. > > Just for me to understand : Unix stores special objects > (such as pipes, sockets, etc.) as file system objects with > access control attached. Does Windows also records special > objects as file system objects ? If so, how are these > objects distinguished from files and directories ? Active Directory objects are stored in a database, not directly on the filesystem. They just happen to both use the security descriptor format. > The purpose of ntfs_valid_descr() is to reject descriptors > which ntfs-3g cannot process properly. I am not keen on > letting special ACEs leaking into translations to Linux. > > Inheritance is of particular concern, because this is rather > complex and there are undocumented special cases which have > to be found by trials and errors. Moreover the result is > generally not satisfactory for Linux users who have > expectations different from Windows ones (typical example : > inheritance of execute permissions). > > Also : if objects can inherit special ACEs from their parent > directory, what prevents them to be inherited to plain files > created in the same directory ? > > Cannot these specific needs be implemented within wimlib ? I am not sure what you mean regarding the purpose of ntfs_valid_descr(), but the existing behavior is that it allows unknown ACEs in both the DACL and SACL except in certain cases. And in those certain cases, such as a callback ACE that is not the last ACE in the list, it is broken for backup/restore applications like wimlib which expect to be able to set security descriptors that were created on Windows. Note that as a backup/restore application, wimlib does not want ACE inheritance at all, nor does it want translation of Windows ACLs to POSIX or Linux permissions. It simply wants to stamp a security descriptor on each file or directory. It can be assumed that the security descriptors are valid in the sense that Windows would accept them. As part of this, it can be assumed that the ACEs in each ACL have the common ACE_HEADER. But it can *not* be assumed that every ACE is of a known type. The expected behavior is that unknown ACEs are settable but are then skipped during access evalution. This would match the Windows behavior. I think Windows users might actually find it quite annoying for Windows to do otherwise because then people could not, for example, restore security descriptors intended for a later version of Windows from a live system running an older version of Windows. Essentially the same argument applies to NTFS-3G. With regards to inheritance, as I pointed out Windows simply performs the standard inheritance algorithm on unknown ACEs per the standard ACE_HEADER flags. It's true that there could be special rules that it is missing, but it seems like the most logical behavior and probably the easiest to implement too. Of course, this isn't currently relevant to wimlib which as I mentioned does not care about inheritance. I just thought I'd suggest it as an improvement (though it's still not clear to me what the current behavior is "supposed" to be, and that's part of the problem). Eric -- The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik ___ ntfs-3g-devel mailing list ntfs-3g-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ntfs-3g-devel