Fix the issue that the gBS->LoadImage pointer was empty.

[jinlun123123123]

The interface shouldn't be replaced at the shim_fini
stage When the vendor certificate doesn't exist.

[kukrimate]

		if (vendor_authorized_size || vendor_deauthorized_size) {
			/*
			 * If shim includes its own certificates then ensure
			 * that anything it boots has performed some
			 * validation of the next image.
			 */
			hook_system_services(systab);
			loader_is_participating = 0;
		}

The above is the hook installation code from shim_init. Hook is indeed only installed when moklist or moklistx is non-empty.

I hope there are no signed shims on the UEFI 3rd party CA with this condition.

EDIT: I initially thought this might be an exploitable UAF, but looks like the pointers written to the system table are always NULL, and I don't think there is any signed UEFI application that would allow an attacker to put shellcode there so maybe it's just an annoying crash.

[xcfxr]

		if (vendor_authorized_size || vendor_deauthorized_size) {
			/*
			 * If shim includes its own certificates then ensure
			 * that anything it boots has performed some
			 * validation of the next image.
			 */
			hook_system_services(systab);
			loader_is_participating = 0;
		}

The above is the hook installation code from shim_init. Hook is indeed only installed when moklist or moklistx is non-empty.

I hope there are no signed shims on the UEFI 3rd party CA with this condition.

EDIT: I initially thought this might be an exploitable UAF, but looks like the pointers written to the system table are always NULL, and I don't think there is any signed UEFI application that would allow an attacker to put shellcode there so maybe it's just an annoying crash.

From the perspective of the logical integrity of the code, shim_fini needs to make the same judgment when uninstalling the hook, so this pr is meaningful.

[kukrimate]

From the perspective of the logical integrity of the code, shim_fini needs to make the same judgment when uninstalling the hook, so this pr is meaningful.

Absolutely, it should be merged, that was more of comment on severity.

[jsetje]

@jinlun123123123 can you please add your signed off by?

[mikebeaton]

Hi - For information, I just checked, and the issue is not that the commit does not have a Signed-off-by, but that the CI DCO check (see Summary at end) apparently requires the Signed-off-by details to exactly match the GitHub account. From my test PR listed there, even a validated alternative address on the GitHub account is not sufficient, it has to match both the name and the account primary email address. (Which seems surprising.)

[jinlun123123123]

Thank you all for reviewing and I resubmitted the PR：
#703

[mikebeaton]

Thank you all for reviewing and I resubmitted the PR： #703

You may already be aware, but again FYI, you can also just force push to your own branch in a PR, and GitHub understands and updates the PR to the new set of commits on the branch.