Author: hbelusca
Date: Sat Apr 5 14:56:41 2014
New Revision: 62623
URL: http://svn.reactos.org/svn/reactos?rev=62623&view=rev
Log:
[RTL]
Some fixes for RtlGetFullPathName_U(str):
- Start to polish RtlpCollapsePath (Work in progress)
- Correctly zero-out the path destination buffer
They fix the following tests:
* ntdll:RtlGetFullPathName_U (2 failures to full success)
* ntdll:RtlGetFullPathName_UstrEx (2 failures to full success)
Modified:
trunk/reactos/lib/rtl/path.c
Modified: trunk/reactos/lib/rtl/path.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/rtl/path.c?rev=62623&r…
==============================================================================
--- trunk/reactos/lib/rtl/path.c [iso-8859-1] (original)
+++ trunk/reactos/lib/rtl/path.c [iso-8859-1] Sat Apr 5 14:56:41 2014
@@ -10,7 +10,7 @@
* Pierre Schweitzer (pierre(a)reactos.org)
*/
-/* INCLUDES *****************************************************************/
+/* INCLUDES *******************************************************************/
#include <rtl.h>
@@ -289,102 +289,137 @@
/******************************************************************
* RtlpCollapsePath (from WINE)
*
- * Helper for RtlGetFullPathName_U.
- * 1) Convert slashes into backslashes
- * 2) Get rid of duplicate backslashes
- * 3) Get rid of . and .. components in the path.
+ * Helper for RtlGetFullPathName_U
+ *
+ * 1) Converts slashes into backslashes and gets rid of duplicated ones;
+ * 2) Gets rid of . and .. components in the path.
+ *
+ * Returns the full path length without its terminating NULL character.
*/
-static VOID
-RtlpCollapsePath(PWSTR path, ULONG mark, BOOLEAN SkipTrailingPathSeparators)
+static ULONG
+RtlpCollapsePath(PWSTR Path, /* ULONG PathBufferSize, ULONG PathLength, */ ULONG mark, BOOLEAN SkipTrailingPathSeparators)
{
PWSTR p, next;
- /* convert every / into a \ */
- for (p = path; *p; p++)
+ // FIXME: Do not suppose NULL-terminated strings!!
+
+ ULONG PathLength = wcslen(Path);
+ PWSTR EndBuffer = Path + PathLength; // Path + PathBufferSize / sizeof(WCHAR);
+ PWSTR EndPath;
+
+ /* Convert slashes into backslashes */
+ for (p = Path; *p; p++)
{
if (*p == L'/') *p = L'\\';
}
- /* collapse duplicate backslashes */
- next = path + max( 1, mark );
+ /* Collapse duplicate backslashes */
+ next = Path + max( 1, mark );
for (p = next; *p; p++)
{
if (*p != L'\\' || next[-1] != L'\\') *next++ = *p;
}
*next = UNICODE_NULL;
-
- p = path + mark;
+ EndPath = next;
+
+ p = Path + mark;
while (*p)
{
if (*p == L'.')
{
- switch(p[1])
+ switch (p[1])
{
case UNICODE_NULL: /* final . */
- if (p > path + mark) p--;
+ if (p > Path + mark) p--;
*p = UNICODE_NULL;
+ EndPath = p;
continue;
+
case L'\\': /* .\ component */
next = p + 2;
- RtlMoveMemory( p, next, (wcslen(next) + 1) * sizeof(WCHAR) );
+ // ASSERT(EndPath - next == wcslen(next));
+ RtlMoveMemory(p, next, (EndPath - next + 1) * sizeof(WCHAR));
+ EndPath -= (next - p);
continue;
+
case L'.':
if (p[2] == L'\\') /* ..\ component */
{
next = p + 3;
- if (p > path + mark)
+ if (p > Path + mark)
{
p--;
- while (p > path + mark && p[-1] != L'\\') p--;
+ while (p > Path + mark && p[-1] != L'\\') p--;
}
- RtlMoveMemory( p, next, (wcslen(next) + 1) * sizeof(WCHAR) );
+ // ASSERT(EndPath - next == wcslen(next));
+ RtlMoveMemory(p, next, (EndPath - next + 1) * sizeof(WCHAR));
+ EndPath -= (next - p);
continue;
}
- else if (!p[2]) /* final .. */
+ else if (p[2] == UNICODE_NULL) /* final .. */
{
- if (p > path + mark)
+ if (p > Path + mark)
{
p--;
- while (p > path + mark && p[-1] != L'\\') p--;
- if (p > path + mark) p--;
+ while (p > Path + mark && p[-1] != L'\\') p--;
+ if (p > Path + mark) p--;
}
*p = UNICODE_NULL;
+ EndPath = p;
continue;
}
break;
}
}
- /* skip to the next component */
+
+ /* Skip to the next component */
while (*p && *p != L'\\') p++;
if (*p == L'\\')
{
- /* remove last dot in previous dir name */
- if (p > path + mark && p[-1] == L'.')
- RtlMoveMemory( p-1, p, (wcslen(p) + 1) * sizeof(WCHAR) );
+ /* Remove last dot in previous dir name */
+ if (p > Path + mark && p[-1] == L'.')
+ {
+ // ASSERT(EndPath - p == wcslen(p));
+ RtlMoveMemory(p - 1, p, (EndPath - p + 1) * sizeof(WCHAR));
+ EndPath--;
+ }
else
+ {
p++;
+ }
}
}
/* Remove trailing backslashes if needed (after the UNC part if it exists) */
if (SkipTrailingPathSeparators)
{
- while (p > path + mark && IS_PATH_SEPARATOR(p[-1])) *p-- = UNICODE_NULL;
+ while (p > Path + mark && IS_PATH_SEPARATOR(p[-1])) p--;
}
/* Remove trailing spaces and dots (for all the path) */
- while (p > path && (p[-1] == L' ' || p[-1] == L'.')) *p-- = UNICODE_NULL;
-
- /* Null-terminate the string */
- *p = UNICODE_NULL;
+ while (p > Path && (p[-1] == L' ' || p[-1] == L'.')) p--;
+
+ /*
+ * Zero-out the discarded buffer zone, starting just after
+ * the path string and going up to the end of the buffer.
+ * It also NULL-terminate the path string.
+ */
+ ASSERT(EndBuffer >= p);
+ RtlZeroMemory(p, (EndBuffer - p + 1) * sizeof(WCHAR));
+
+ /* Return the real path length */
+ PathLength = (p - Path);
+ // ASSERT(PathLength == wcslen(Path));
+ return (PathLength * sizeof(WCHAR));
}
/******************************************************************
* RtlpSkipUNCPrefix (from WINE)
*
* Helper for RtlGetFullPathName_U
- * Skip the \\share\dir part of a file name and return the new position
- * (which can point on the last backslash of "dir\")
+ *
+ * Skips the \\share\dir part of a file name and returns the new position
+ * (which can point on the last backslash of "dir\").
*/
static SIZE_T
RtlpSkipUNCPrefix(PCWSTR FileNameBuffer)
@@ -542,7 +577,7 @@
return FullLength + sizeof(UNICODE_NULL);
}
- /* Zero out the destination buffer. FileName must be different from Buffer */
+ /* Zero-out the destination buffer. FileName must be different from Buffer */
RtlZeroMemory(Buffer, Size);
/* Get the path type */
@@ -551,7 +586,7 @@
/**********************************************
- ** CODE REWRITTEN IS HAPPENING THERE **
+ ** CODE REWRITING IS HAPPENING THERE **
**********************************************/
Source = FileNameBuffer;
SourceLength = FileNameLength;
@@ -619,7 +654,7 @@
* FIXME: Win2k seems to check that the environment
* variable actually points to an existing directory.
* If not, root of the drive is used (this seems also
- * to be the only spot in RtlGetFullPathName that the
+ * to be the only place in RtlGetFullPathName that the
* existence of a part of a path is checked).
*/
EnvVarValue.Buffer[EnvVarValue.Length / sizeof(WCHAR)] = L'\\';
@@ -709,13 +744,10 @@
/*
* Build the full path
*/
- // if (ShortName) DPRINT1("buffer(1) = %S\n", Buffer);
/* Copy the path's prefix */
if (PrefixLength) RtlMoveMemory(Buffer, Prefix, PrefixLength);
- // if (ShortName) DPRINT1("buffer(2) = %S\n", Buffer);
/* Copy the remaining part of the path */
RtlMoveMemory(Buffer + PrefixLength / sizeof(WCHAR), Source, SourceLength + sizeof(WCHAR));
- // if (ShortName) DPRINT1("buffer(3) = %S\n", Buffer);
/* Some cleanup */
Prefix = NULL;
@@ -726,15 +758,11 @@
}
/*
- * Finally, put the path in canonical form,
- * i.e. simplify redundant . and .., etc...
+ * Finally, put the path in canonical form (remove redundant . and ..,
+ * (back)slashes...) and retrieve the length of the full path name
+ * (without its terminating null character) (in chars).
*/
- // if (*PathType == RtlPathTypeUncAbsolute) DPRINT1("RtlpCollapsePath('%S', %lu)\n", Buffer, PrefixCut);
- RtlpCollapsePath(Buffer, PrefixCut, SkipTrailingPathSeparators);
- // if (ShortName) DPRINT1("buffer(4) = %S\n", Buffer);
-
- /* Get the length of the full path name, without its terminating null character */
- reqsize = wcslen(Buffer) * sizeof(WCHAR);
+ reqsize = RtlpCollapsePath(Buffer, /* Size, reqsize, */ PrefixCut, SkipTrailingPathSeparators);
/* Find the file part, which is present after the last path separator */
if (ShortName)
@@ -761,7 +789,8 @@
Quit:
/* Release PEB lock */
RtlReleasePebLock();
- return (ULONG)reqsize;
+
+ return reqsize;
}
NTSTATUS
Author: pschweitzer
Date: Sat Apr 5 13:13:01 2014
New Revision: 62618
URL: http://svn.reactos.org/svn/reactos?rev=62618&view=rev
Log:
[RTL]
Properly check for total length in LdrpGetProcedureAddress().
It contains more than just a name.
Fixes a buffer overrun.
CID #716122
Modified:
trunk/reactos/dll/ntdll/ldr/ldrutils.c
Modified: trunk/reactos/dll/ntdll/ldr/ldrutils.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/ntdll/ldr/ldrutils.c?r…
==============================================================================
--- trunk/reactos/dll/ntdll/ldr/ldrutils.c [iso-8859-1] (original)
+++ trunk/reactos/dll/ntdll/ldr/ldrutils.c [iso-8859-1] Sat Apr 5 13:13:01 2014
@@ -2267,7 +2267,7 @@
}
/* Check if our buffer is large enough */
- if (Name->Length > sizeof(ImportBuffer))
+ if (Length > sizeof(ImportBuffer))
{
/* Allocate from heap, plus 2 bytes for the Hint */
ImportName = RtlAllocateHeap(RtlGetProcessHeap(),