From 4a9b204920152c668228a9d43a63be39b0c32f45 Mon Sep 17 00:00:00 2001 From: Matthias Aßhauer Date: Sat, 8 Jan 2022 16:02:30 +0000 Subject: lazyload: use correct calling conventions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Christoph Reiter reported on the Git for Windows issue tracker[1], that mingw_strftime() imports strftime() from ucrtbase.dll with the wrong calling convention. It should be __cdecl instead of WINAPI, which we always use in DECLARE_PROC_ADDR(). The MSYS2 project encountered cmake sefaults on x86 Windows caused by the same issue in the cmake source. [2] There are no known git crashes that where caused by this, yet, but we should try to prevent them. We import two other non-WINAPI functions via DECLARE_PROC_ADDR(), too. * NtSetSystemInformation() (NTAPI) * GetUserNameExW() (SEC_ENTRY) NTAPI, SEC_ENTRY and WINAPI are all ususally defined as __stdcall, but there are circumstances where they're defined differently. Teach DECLARE_PROC_ADDR() about calling conventions and be explicit about when we want to use which calling convention. Import winnt.h for the definition of NTAPI and sspi.h for SEC_ENTRY near their respective only users. [1] https://github.com/git-for-windows/git/issues/3560 [2] https://github.com/msys2/MINGW-packages/issues/10152 Reported-By: Christoph Reiter Signed-off-by: Matthias Aßhauer Signed-off-by: Junio C Hamano --- t/helper/test-drop-caches.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 't') diff --git a/t/helper/test-drop-caches.c b/t/helper/test-drop-caches.c index 7b4278462b..e37396dd9c 100644 --- a/t/helper/test-drop-caches.c +++ b/t/helper/test-drop-caches.c @@ -3,6 +3,7 @@ #if defined(GIT_WINDOWS_NATIVE) #include "lazyload.h" +#include static int cmd_sync(void) { @@ -86,7 +87,8 @@ static int cmd_dropcaches(void) { HANDLE hProcess = GetCurrentProcess(); HANDLE hToken; - DECLARE_PROC_ADDR(ntdll.dll, DWORD, NtSetSystemInformation, INT, PVOID, ULONG); + DECLARE_PROC_ADDR(ntdll.dll, DWORD, NTAPI, NtSetSystemInformation, INT, PVOID, + ULONG); SYSTEM_MEMORY_LIST_COMMAND command; int status; -- cgit v1.3 From c39fc06b999305963600358f3f5e99698440cad2 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 10 Jan 2022 16:19:06 -0500 Subject: fmt-merge-msg: prevent use-after-free with signed tags When merging a signed tag, fmt_merge_msg_sigs() is responsible for populating the body of the merge message with the names of the signed tags, their signatures, and the validity of those signatures. In 02769437e1 (ssh signing: use sigc struct to pass payload, 2021-12-09), check_signature() was taught to pass the object payload via the sigc struct instead of passing the payload buffer separately. In effect, 02769437e1 causes buf, and sigc.payload to point at the same region in memory. This causes a problem for fmt_tag_signature(), which wants to read from this location, since it is freed beforehand by signature_check_clear() (which frees it via sigc's `payload` member). That makes the subsequent use in fmt_tag_signature() a use-after-free. As a result, merge messages did not contain the body of any signed tags. Luckily, they tend not to contain garbage, either, since the result of strstr()-ing the object buffer in fmt_tag_signature() is guarded: const char *tag_body = strstr(buf, "\n\n"); if (tag_body) { tag_body += 2; strbuf_add(tagbuf, tag_body, buf + len - tag_body); } Unfortunately, the tests in t6200 did not catch this at the time because they do not search for the body of signed tags in fmt-merge-msg's output. Resolve this by waiting to call signature_check_clear() until after its contents can be safely discarded. Harden ourselves against any future regressions in this area by making sure we can find signed tag messages in the output of fmt-merge-msg, too. Reported-by: Linus Torvalds Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- fmt-merge-msg.c | 2 +- t/t6200-fmt-merge-msg.sh | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) (limited to 't') diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c index e4f7810be2..422bd0c055 100644 --- a/fmt-merge-msg.c +++ b/fmt-merge-msg.c @@ -541,7 +541,6 @@ static void fmt_merge_msg_sigs(struct strbuf *out) else strbuf_addstr(&sig, sigc.output); } - signature_check_clear(&sigc); if (!tag_number++) { fmt_tag_signature(&tagbuf, &sig, buf, len); @@ -565,6 +564,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out) } strbuf_release(&payload); strbuf_release(&sig); + signature_check_clear(&sigc); next: free(origbuf); } diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh index 6e10a539ce..d3e847f5e2 100755 --- a/t/t6200-fmt-merge-msg.sh +++ b/t/t6200-fmt-merge-msg.sh @@ -126,6 +126,7 @@ test_expect_success GPG 'message for merging local tag signed by good key' ' git fetch . signed-good-tag && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}signed-good-tag${apos}" actual && + grep "^signed-tag-msg" actual && grep "^# gpg: Signature made" actual && grep "^# gpg: Good signature from" actual ' @@ -135,6 +136,7 @@ test_expect_success GPG 'message for merging local tag signed by unknown key' ' git fetch . signed-good-tag && GNUPGHOME=. git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}signed-good-tag${apos}" actual && + grep "^signed-tag-msg" actual && grep "^# gpg: Signature made" actual && grep -E "^# gpg: Can${apos}t check signature: (public key not found|No public key)" actual ' @@ -145,6 +147,7 @@ test_expect_success GPGSSH 'message for merging local tag signed by good ssh key git fetch . signed-good-ssh-tag && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}signed-good-ssh-tag${apos}" actual && + grep "^signed-ssh-tag-msg" actual && grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual && ! grep "${GPGSSH_BAD_SIGNATURE}" actual ' @@ -155,6 +158,7 @@ test_expect_success GPGSSH 'message for merging local tag signed by unknown ssh git fetch . signed-untrusted-ssh-tag && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}signed-untrusted-ssh-tag${apos}" actual && + grep "^signed-ssh-tag-msg-untrusted" actual && grep "${GPGSSH_GOOD_SIGNATURE_UNTRUSTED}" actual && ! grep "${GPGSSH_BAD_SIGNATURE}" actual && grep "${GPGSSH_KEY_NOT_TRUSTED}" actual @@ -166,6 +170,7 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign git fetch . expired-signed && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}expired-signed${apos}" actual && + grep "^expired-signed" actual && ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual ' @@ -175,6 +180,7 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign git fetch . notyetvalid-signed && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}notyetvalid-signed${apos}" actual && + grep "^notyetvalid-signed" actual && ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual ' @@ -184,6 +190,7 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign git fetch . timeboxedvalid-signed && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}timeboxedvalid-signed${apos}" actual && + grep "^timeboxedvalid-signed" actual && grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual && ! grep "${GPGSSH_BAD_SIGNATURE}" actual ' @@ -194,6 +201,7 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign git fetch . timeboxedinvalid-signed && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}timeboxedinvalid-signed${apos}" actual && + grep "^timeboxedinvalid-signed" actual && ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual ' -- cgit v1.3 From 0517f591ca290a14ee3e516e478e8d2b78b45822 Mon Sep 17 00:00:00 2001 From: Fabian Stelzer Date: Wed, 12 Jan 2022 13:07:57 +0100 Subject: t/gpg: simplify test for unknown key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To test for a key that is completely unknown to the keyring we need one to sign the commit with. This was done by generating a new key and not add it into the keyring. To avoid the key generation overhead and problems where GPG did hang in CI during it, switch GNUPGHOME to the empty $GNUPGHOME_NOT_USED instead, therefore making all used keys unknown for this single `verify-commit` call. Reported-by: Ævar Arnfjörð Bjarmason Signed-off-by: Fabian Stelzer Reviewed-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/t7510-signed-commit.sh | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) (limited to 't') diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index d65a0171f2..50721aaf79 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -71,25 +71,7 @@ test_expect_success GPG 'create signed commits' ' git tag eleventh-signed $(cat oid) && echo 12 | git commit-tree --gpg-sign=B7227189 HEAD^{tree} >oid && test_line_count = 1 oid && - git tag twelfth-signed-alt $(cat oid) && - - cat >keydetails <<-\EOF && - Key-Type: RSA - Key-Length: 2048 - Subkey-Type: RSA - Subkey-Length: 2048 - Name-Real: Unknown User - Name-Email: unknown@git.com - Expire-Date: 0 - %no-ask-passphrase - %no-protection - EOF - gpg --batch --gen-key keydetails && - echo 13 >file && git commit -a -S"unknown@git.com" -m thirteenth && - git tag thirteenth-signed && - DELETE_FINGERPRINT=$(gpg -K --with-colons --fingerprint --batch unknown@git.com | grep "^fpr" | head -n 1 | awk -F ":" "{print \$10;}") && - gpg --batch --yes --delete-secret-keys $DELETE_FINGERPRINT && - gpg --batch --yes --delete-keys unknown@git.com + git tag twelfth-signed-alt $(cat oid) ' test_expect_success GPG 'verify and show signatures' ' @@ -129,7 +111,7 @@ test_expect_success GPG 'verify and show signatures' ' ' test_expect_success GPG 'verify-commit exits failure on unknown signature' ' - test_must_fail git verify-commit thirteenth-signed 2>actual && + test_must_fail env GNUPGHOME="$GNUPGHOME_NOT_USED" git verify-commit initial 2>actual && ! grep "Good signature from" actual && ! grep "BAD signature from" actual && grep -q -F -e "No public key" -e "public key not found" actual -- cgit v1.3 From 59069107948bc87b8b6d46d49a52df410c4a8745 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Thu, 13 Jan 2022 21:28:45 +0100 Subject: t1450-fsck: exec-bit is not needed to make loose object writable A test case wants to append stuff to a loose object file to ensure that this kind of corruption is detected. To make a read-only loose object file writable with chmod, it is not necessary to also make it executable. Replace the bitmask 755 with the instruction +w to request only the write bit and to also heed the umask. And get rid of a POSIXPERM prerequisite, which is unnecessary for the test. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t1450-fsck.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 6337236fd8..de50c0ea01 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -94,13 +94,13 @@ test_expect_success 'object with hash and type mismatch' ' ) ' -test_expect_success POSIXPERM 'zlib corrupt loose object output ' ' +test_expect_success 'zlib corrupt loose object output ' ' git init --bare corrupt-loose-output && ( cd corrupt-loose-output && oid=$(git hash-object -w --stdin --literally >$oidf && cat >expect.error <<-EOF && -- cgit v1.3