From 6daaba3a508f091da72a0a74dfd7415747a8a639 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 16 Feb 2025 12:45:57 -0500 Subject: [PATCH 1/3] Use `br` literals to express `\` paths more clearly in tests In `gix-validate` tests that Windows-style paths are validated correctly, this changes byte strings that contain `\` to be raw byte string literals (`rb`) instead of non-raw byte string literals (`b`). This is to improve readability given that some literals contain consecutive backslashes: this makes it easier to tell if, and how many, consecutive backslashes there are. --- gix-validate/tests/path/mod.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/gix-validate/tests/path/mod.rs b/gix-validate/tests/path/mod.rs index 84950e787ca..6a5bd338deb 100644 --- a/gix-validate/tests/path/mod.rs +++ b/gix-validate/tests/path/mod.rs @@ -53,10 +53,10 @@ mod component { mktest!(ascii, b"ascii-only_and-that"); mktest!(unicode, "😁👍👌".as_bytes()); - mktest!(backslashes_on_unix, b"\\", UNIX_OPTS); + mktest!(backslashes_on_unix, br"\", UNIX_OPTS); mktest!(drive_letters_on_unix, b"c:", UNIX_OPTS); mktest!(virtual_drive_letters_on_unix, "֍:".as_bytes(), UNIX_OPTS); - mktest!(unc_path_on_unix, b"\\\\?\\pictures", UNIX_OPTS); + mktest!(unc_path_on_unix, br"\\?\pictures", UNIX_OPTS); mktest!(not_dot_git_longer, b".gitu", NO_OPTS); mktest!(not_dot_git_longer_all, b".gitu"); mktest!(not_dot_gitmodules_shorter, b".gitmodule", Symlink, NO_OPTS); @@ -66,7 +66,7 @@ mod component { mktest!(dot_gitmodules_as_file, b".gitmodules", UNIX_OPTS); mktest!( starts_with_dot_git_with_backslashes_on_linux, - b".git\\hooks\\pre-commit", + br".git\hooks\pre-commit", UNIX_OPTS ); mktest!(not_dot_git_shorter, b".gi", NO_OPTS); @@ -136,7 +136,7 @@ mod component { mktest!(dot_git_upper, b".GIT", Error::DotGitDir, NO_OPTS); mktest!( starts_with_dot_git_with_backslashes_on_windows, - b".git\\hooks\\pre-commit", + br".git\hooks\pre-commit", Error::PathSeparator ); mktest!(dot_git_upper_hfs, ".GIT\u{200e}".as_bytes(), Error::DotGitDir); @@ -225,10 +225,10 @@ mod component { mktest!(path_separator_slash_trailing, b"a/", Error::PathSeparator); mktest!(path_separator_slash_only, b"/", Error::PathSeparator); mktest!(slashes_on_windows, b"/", Error::PathSeparator, ALL_OPTS); - mktest!(backslashes_on_windows, b"\\", Error::PathSeparator, ALL_OPTS); - mktest!(path_separator_backslash_between, b"a\\b", Error::PathSeparator); - mktest!(path_separator_backslash_leading, b"\\a", Error::PathSeparator); - mktest!(path_separator_backslash_trailing, b"a\\", Error::PathSeparator); + mktest!(backslashes_on_windows, br"\", Error::PathSeparator, ALL_OPTS); + mktest!(path_separator_backslash_between, br"a\b", Error::PathSeparator); + mktest!(path_separator_backslash_leading, br"\a", Error::PathSeparator); + mktest!(path_separator_backslash_trailing, br"a\", Error::PathSeparator); mktest!(aux_mixed, b"Aux", Error::WindowsReservedName); mktest!(aux_with_extension, b"aux.c", Error::WindowsReservedName); mktest!(com_lower, b"com1", Error::WindowsReservedName); @@ -261,7 +261,7 @@ mod component { Error::WindowsPathPrefix, ALL_OPTS ); - mktest!(unc_path, b"\\\\?\\pictures", Error::PathSeparator, ALL_OPTS); + mktest!(unc_path, br"\\?\pictures", Error::PathSeparator, ALL_OPTS); #[test] fn ntfs_gitmodules() { From f5ef10d30e500f12ff96ea1f4a9337e12c72ebbb Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 16 Feb 2025 13:14:08 -0500 Subject: [PATCH 2/3] Test Windows `.middle.extension` filenames When `` is a reserved device name on Windows like `NUL`, in addition to filenames where it is followed by an extension being treated specially in the same way (`.extension`), filenames where the operation of repeatedly removing extensions would produce `` (`.middle.extension`, `.middle1.middle2.extension`, and so forth) are also treated by Windows like the reserved name. The existing code in `gix-validate`, and thus the validation of paths and refs that checks for reserved Windows device names, already does the right thing here. While the existing tests are presented as being for extensions, the implementation treats a name as reserved when it is parsed as a reserved name followed by a `.` (among other cases), regardless of whether what comes after the `.` is actually an extension in the strict sense. Therefore this is just adding regression tests to express the rule more clearly and, more importantly, to protect against accidental breakage in future changes. (Because of the way gitoxide uses this validation, suhc breakage would likely introduce a vulnerability similar to CVE-2024-35197, because this is part of how we safeguard against malicious names when cloning untrusted remote repos.) The idea to test this is inspired by a new check in `dunce`: https://gitlab.com/kornelski/dunce/-/commit/0ceb0ae141bf78c6d9d68488a55d22cd94658339 --- gix-validate/tests/path/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/gix-validate/tests/path/mod.rs b/gix-validate/tests/path/mod.rs index 6a5bd338deb..727c32ad279 100644 --- a/gix-validate/tests/path/mod.rs +++ b/gix-validate/tests/path/mod.rs @@ -1,9 +1,9 @@ #[test] fn component_is_windows_device() { - for device in ["con", "CONIN$", "lpt1.txt", "AUX", "Prn", "NUL", "COM9"] { + for device in ["con", "CONIN$", "lpt1.txt", "AUX", "Prn", "NUL", "COM9", "nul.a.b "] { assert!(gix_validate::path::component_is_windows_device(device.into())); } - for not_device in ["coni", "CONIN", "lpt", "AUXi", "aPrn", "NULl", "COM"] { + for not_device in ["coni", "CONIN", "lpt", "AUXi", "aPrn", "NULl", "COM", "a.nul.b "] { assert!(!gix_validate::path::component_is_windows_device(not_device.into())); } } @@ -82,6 +82,9 @@ mod component { mktest!(conin_without_dollar, b"conin"); mktest!(not_con, b"com"); mktest!(also_not_con, b"co"); + mktest!(con_as_middle, b"x.CON.zip"); + mktest!(con_after_space, b" CON"); + mktest!(con_after_space_mixed, b" coN.tar.xz"); mktest!(not_nul, b"null"); mktest!( not_dot_gitmodules_shorter_hfs, @@ -248,6 +251,8 @@ mod component { mktest!(prn_mixed_with_extension, b"PrN.abc", Error::WindowsReservedName); mktest!(con, b"CON", Error::WindowsReservedName); mktest!(con_with_extension, b"CON.abc", Error::WindowsReservedName); + mktest!(con_with_middle, b"CON.tar.xz", Error::WindowsReservedName); + mktest!(con_mixed_with_middle, b"coN.tar.xz ", Error::WindowsReservedName); mktest!( conout_mixed_with_extension, b"ConOut$ .xyz", From eaa59a397db2e2bebb9c23cc06b9a27b4010fcdd Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 16 Feb 2025 13:48:24 -0500 Subject: [PATCH 3/3] Test other UNC-like prefixes besides `\\?\` The test cases added in this commit are not very high value, since any `\` in what is supposed to be a component should be detected as invalid on Windows, which entails that these paths are detected as such. Adding these is mainly to clarify that the `\\?\` is not special here, comparedd to other kinds of `\` and `\\` paths. --- gix-validate/tests/path/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gix-validate/tests/path/mod.rs b/gix-validate/tests/path/mod.rs index 727c32ad279..1330ff4a9c7 100644 --- a/gix-validate/tests/path/mod.rs +++ b/gix-validate/tests/path/mod.rs @@ -266,7 +266,10 @@ mod component { Error::WindowsPathPrefix, ALL_OPTS ); + mktest!(unc_net_path, br"\\host", Error::PathSeparator, ALL_OPTS); mktest!(unc_path, br"\\?\pictures", Error::PathSeparator, ALL_OPTS); + mktest!(unc_device_path, br"\\.\pictures", Error::PathSeparator, ALL_OPTS); + mktest!(unc_nt_obj_path, br"\??\pictures", Error::PathSeparator, ALL_OPTS); #[test] fn ntfs_gitmodules() {