From e2f63788470a1cdd2d591505a00dd13ff1bf6a34 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 12 Mar 2014 12:01:56 -0700 Subject: [PATCH] Change DirCacheCheckout to verify path using ObjectChecker Reuse the generic logic in ObjectChecker to examine paths. This required extracting the scanner loop to check for bad characters within the path name segment. Change-Id: I02e964d114fb544a0c1657790d5367c3a2b09dff --- .../eclipse/jgit/lib/ObjectCheckerTest.java | 10 ++ .../jgit/dircache/DirCacheCheckout.java | 151 ++++-------------- .../org/eclipse/jgit/lib/ObjectChecker.java | 67 +++++--- 3 files changed, 83 insertions(+), 145 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java index 8cdb0ae350..06745650ca 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java @@ -1449,6 +1449,16 @@ public class ObjectCheckerTest { } } + @Test + public void testRejectNulInPathSegment() { + try { + checker.checkPathSegment(Constants.encodeASCII("a\u0000b"), 0, 3); + fail("incorrectly accepted NUL in middle of name"); + } catch (CorruptObjectException e) { + assertEquals("name contains byte 0x00", e.getMessage()); + } + } + @Test public void testRejectSpaceAtEndOnWindows() { checker.setSafeForWindows(true); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java index 44b4276905..5275b4c898 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java @@ -62,6 +62,7 @@ import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.CoreConfig.AutoCRLF; import org.eclipse.jgit.lib.CoreConfig.SymLinks; import org.eclipse.jgit.lib.FileMode; +import org.eclipse.jgit.lib.ObjectChecker; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.ObjectReader; @@ -1164,26 +1165,13 @@ public class DirCacheCheckout { entry.setLength((int) ol.getSize()); } - private static byte[][] forbidden; - static { - String[] list = getSortedForbiddenFileNames(); - forbidden = new byte[list.length][]; - for (int i = 0; i < list.length; ++i) - forbidden[i] = Constants.encodeASCII(list[i]); - } - - static String[] getSortedForbiddenFileNames() { - String[] list = new String[] { "AUX", "COM1", "COM2", "COM3", "COM4", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ - "COM5", "COM6", "COM7", "COM8", "COM9", "CON", "LPT1", "LPT2", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$ //$NON-NLS-7$ //$NON-NLS-8$ - "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", "NUL", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$ //$NON-NLS-7$ //$NON-NLS-8$ - "PRN" }; //$NON-NLS-1$ - return list; - } - private static void checkValidPath(CanonicalTreeParser t) throws InvalidPathException { + ObjectChecker chk = new ObjectChecker() + .setSafeForWindows(SystemReader.getInstance().isWindows()) + .setSafeForMacOS(SystemReader.getInstance().isMacOS()); for (CanonicalTreeParser i = t; i != null; i = i.getParent()) - checkValidPathSegment(i); + checkValidPathSegment(chk, i); } /** @@ -1195,119 +1183,36 @@ public class DirCacheCheckout { * @since 3.3 */ public static void checkValidPath(String path) throws InvalidPathException { - boolean isWindows = SystemReader.getInstance().isWindows(); - boolean isOSX = SystemReader.getInstance().isMacOS(); - boolean ignCase = isOSX || isWindows; + ObjectChecker chk = new ObjectChecker() + .setSafeForWindows(SystemReader.getInstance().isWindows()) + .setSafeForMacOS(SystemReader.getInstance().isMacOS()); byte[] bytes = Constants.encode(path); int segmentStart = 0; - for (int i = 0; i < bytes.length; i++) { - if (bytes[i] == '/') { - checkValidPathSegment(isWindows, ignCase, bytes, segmentStart, - i, path); - segmentStart = i + 1; - } - } - if (segmentStart < bytes.length) - checkValidPathSegment(isWindows, ignCase, bytes, segmentStart, - bytes.length, path); - } - - private static void checkValidPathSegment(CanonicalTreeParser t) - throws InvalidPathException { - boolean isWindows = SystemReader.getInstance().isWindows(); - boolean isOSX = SystemReader.getInstance().isMacOS(); - boolean ignCase = isOSX || isWindows; - - int ptr = t.getNameOffset(); - byte[] raw = t.getEntryPathBuffer(); - int end = ptr + t.getNameLength(); - - checkValidPathSegment(isWindows, ignCase, raw, ptr, end, - t.getEntryPathString()); - } - - private static void checkValidPathSegment(boolean isWindows, - boolean ignCase, byte[] raw, int ptr, int end, String path) { - // Validate path component at this level of the tree - int start = ptr; - while (ptr < end) { - if (raw[ptr] == '/') - throw new InvalidPathException( - JGitText.get().invalidPathContainsSeparator, "/", path); //$NON-NLS-1$ - if (isWindows) { - if (raw[ptr] == '\\') - throw new InvalidPathException( - JGitText.get().invalidPathContainsSeparator, - "\\", path); //$NON-NLS-1$ - if (raw[ptr] == ':') - throw new InvalidPathException( - JGitText.get().invalidPathContainsSeparator, - ":", path); //$NON-NLS-1$ - } - ptr++; - } - // '.' and '..' are invalid here - if (ptr - start == 1) { - if (raw[start] == '.') - throw new InvalidPathException(path); - } else if (ptr - start == 2) { - if (raw[start] == '.') - if (raw[start + 1] == '.') - throw new InvalidPathException(path); - } else if (ptr - start == 4) { - // .git (possibly case insensitive) is disallowed - if (raw[start] == '.') - if (raw[start + 1] == 'g' || (ignCase && raw[start + 1] == 'G')) - if (raw[start + 2] == 'i' - || (ignCase && raw[start + 2] == 'I')) - if (raw[start + 3] == 't' - || (ignCase && raw[start + 3] == 'T')) - throw new InvalidPathException(path); - } - if (isWindows) { - // Space or period at end of file name is ignored by Windows. - // Treat this as a bad path for now. We may want to handle - // this as case insensitivity in the future. - if (ptr > 0) { - if (raw[ptr - 1] == '.') - throw new InvalidPathException( - JGitText.get().invalidPathPeriodAtEndWindows, path); - if (raw[ptr - 1] == ' ') - throw new InvalidPathException( - JGitText.get().invalidPathSpaceAtEndWindows, path); - } - - int i; - // Bad names, eliminate suffix first - for (i = start; i < ptr; ++i) - if (raw[i] == '.') - break; - int len = i - start; - if (len == 3 || len == 4) { - for (int j = 0; j < forbidden.length; ++j) { - if (forbidden[j].length == len) { - if (toUpper(raw[start]) < forbidden[j][0]) - break; - int k; - for (k = 0; k < len; ++k) { - if (toUpper(raw[start + k]) != forbidden[j][k]) - break; - } - if (k == len) - throw new InvalidPathException( - JGitText.get().invalidPathReservedOnWindows, - RawParseUtils.decode(forbidden[j]), path); - } + try { + for (int i = 0; i < bytes.length; i++) { + if (bytes[i] == '/') { + chk.checkPathSegment(bytes, segmentStart, i); + segmentStart = i + 1; } } + chk.checkPathSegment(bytes, segmentStart, bytes.length); + } catch (CorruptObjectException e) { + throw new InvalidPathException(e.getMessage()); } } - private static byte toUpper(byte b) { - if (b >= 'a' && b <= 'z') - return (byte) (b - ('a' - 'A')); - return b; + private static void checkValidPathSegment(ObjectChecker chk, + CanonicalTreeParser t) throws InvalidPathException { + try { + int ptr = t.getNameOffset(); + int end = ptr + t.getNameLength(); + chk.checkPathSegment(t.getEntryPathBuffer(), ptr, end); + } catch (CorruptObjectException err) { + String path = t.getEntryPathString(); + InvalidPathException i = new InvalidPathException(path); + i.initCause(err); + throw i; + } } - } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java index dd430024b0..38fd0a1eb6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java @@ -369,44 +369,67 @@ public class ObjectChecker { throw new CorruptObjectException("invalid mode " + thisMode); final int thisNameB = ptr; - for (;;) { - if (ptr == sz) - throw new CorruptObjectException("truncated in name"); - final byte c = raw[ptr++]; - if (c == 0) - break; - if (c == '/') - throw new CorruptObjectException("name contains '/'"); - if (windows && isInvalidOnWindows(c)) { - if (c > 31) - throw new CorruptObjectException(String.format( - "name contains '%c'", c)); - throw new CorruptObjectException(String.format( - "name contains byte 0x%x", c & 0xff)); - } - } - checkPathSegment(raw, thisNameB, ptr - 1); - if (duplicateName(raw, thisNameB, ptr - 1)) + ptr = scanPathSegment(raw, ptr, sz); + if (ptr == sz || raw[ptr] != 0) + throw new CorruptObjectException("truncated in name"); + checkPathSegment2(raw, thisNameB, ptr); + if (duplicateName(raw, thisNameB, ptr)) throw new CorruptObjectException("duplicate entry names"); if (lastNameB != 0) { final int cmp = pathCompare(raw, lastNameB, lastNameE, - lastMode, thisNameB, ptr - 1, thisMode); + lastMode, thisNameB, ptr, thisMode); if (cmp > 0) throw new CorruptObjectException("incorrectly sorted"); } lastNameB = thisNameB; - lastNameE = ptr - 1; + lastNameE = ptr; lastMode = thisMode; - ptr += Constants.OBJECT_ID_LENGTH; + ptr += 1 + Constants.OBJECT_ID_LENGTH; if (ptr > sz) throw new CorruptObjectException("truncated in object id"); } } - private void checkPathSegment(byte[] raw, int ptr, int end) + private int scanPathSegment(byte[] raw, int ptr, int end) + throws CorruptObjectException { + for (; ptr < end; ptr++) { + byte c = raw[ptr]; + if (c == 0) + return ptr; + if (c == '/') + throw new CorruptObjectException("name contains '/'"); + if (windows && isInvalidOnWindows(c)) { + if (c > 31) + throw new CorruptObjectException(String.format( + "name contains '%c'", c)); + throw new CorruptObjectException(String.format( + "name contains byte 0x%x", c & 0xff)); + } + } + return ptr; + } + + /** + * Check tree path entry for validity. + * + * @param raw buffer to scan. + * @param ptr offset to first byte of the name. + * @param end offset to one past last byte of name. + * @throws CorruptObjectException name is invalid. + * @since 3.4 + */ + public void checkPathSegment(byte[] raw, int ptr, int end) + throws CorruptObjectException { + int e = scanPathSegment(raw, ptr, end); + if (e < end && raw[e] == 0) + throw new CorruptObjectException("name contains byte 0x00"); + checkPathSegment2(raw, ptr, end); + } + + private void checkPathSegment2(byte[] raw, int ptr, int end) throws CorruptObjectException { if (ptr == end) throw new CorruptObjectException("zero length name"); -- 2.39.5