Skip to content
This repository has been archived by the owner on May 22, 2023. It is now read-only.

Introduce a new mitigation #31

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Introduce a new mitigation #31

wants to merge 1 commit into from

Conversation

LIJI32
Copy link

@LIJI32 LIJI32 commented Aug 2, 2022

Hi, I'd like to introduce a new mitigation to the XNU kernel.

Pros:

  • Prevents all userspace to kernel PEs
  • Mitigates all in-userspace vulnerabilities
  • Eliminates 99% of the remaining kernel vulnerabilities
  • No performance costs

Cons:

  • Drops support for certain rarely-used power user features such as process creation and communication with hardware

Judging by Apple's recent developments in security, such as APFS seals that completely prevent modification of system files, new code signing policies that are hostile towards indie and open-source developers, and untested overly-strict sandbox profiles that break operating system functionality, I believe this is a welcome change that respects Apple's security-above-all policy.

@PayterX
Copy link

PayterX commented Aug 14, 2022

Still leaves too many opportunities for code execution, not strict enough for Apple.

Copy link

@BlueCannonBall BlueCannonBall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the changes are of sound origin and their author is clearly of sound body, mind, and intention. These changes align very well with well-established Apple policies relating to security.

Copy link

@Altanis Altanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link

@UE2020 UE2020 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is brilliant!

@Nul-led
Copy link

Nul-led commented Aug 23, 2022

image.jpg

Copy link

@Nul-led Nul-led left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is obviously a malicious attempt to undermine Apple's recently changed policies and thus cannot be approved as is.

Copy link

@aaaaaa123456789 aaaaaa123456789 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a clear defense-in-depth mitigation that will prevent future zero-day exploits from compromising users' security and privacy. Implementing policies discovered by previous research projects, this is the kind of revolutionary improvement that should showcase any significant change in security policy.
In a world where not even CPUs can be trusted to be secure, this is exactly what the average layperson needs to keep their system secure without needing the constant assistance of highly-skilled and highly-specialized professionals.

A bold move, but a necessary one, and maybe the one that will even push power users like myself to consider using Apple products as daily drivers. Big thumbs up.

@twisted-nematic57
Copy link

You are a legend.

Copy link

@ivoszbg ivoszbg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's only one word that can describe this correctly:
Brilliant.

@twisted-nematic57
Copy link

Ah yes, that was the word I was looking for.

Copy link

@xerz-one xerz-one left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exception incorrectly managed. The current implementation of the PR calls the panic procedure, which is a known unsafe function that leads to undefined behavior, as it then proceeds to execute Turing-complete instructions.

I suggest, instead, to make a new PR which completely gets rid of the faulty code. This includes anything which reads or writes into CPU registers, such as the PC or the IR.

@twisted-nematic57
Copy link

Eliminates 99% of the remaining kernel vulnerabilities

$2,000,000 bug bounty for figuring out that 1%!

/s LOL

Copy link

@yyogo yyogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Scherso
Copy link

Scherso commented Jan 19, 2023

LGTM!

1 similar comment
@Glowman554
Copy link

LGTM!

Copy link

@IsaccBarker IsaccBarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the message may be a bit too vague here. What about "malicious code execution prevented"? I feel like this can provide the user with just enough information to let them feel somewhat informed while still being very vague, which is a hallmark of Apple systems.

I also want to bring up that this text won't actually be visible, I think, until after console_init() is called on line 417. Otherwise, it'll just be logged to somewhere that might not even be accessible with a system that is incapable of executing code of much use at all.

@RoseApollo
Copy link

"malicious code execution prevented" is way too descriptive, I think "legacy execution system has been deprecated" works much better, as it perfectly describes to a user what is going on, and how it can be fixed, just like any other issue on Darwin / macOS.

@9021007
Copy link

9021007 commented Apr 26, 2023

This is truly a pull request of all time

@IsaccBarker
Copy link

"malicious code execution prevented" is way too descriptive, I think "legacy execution system has been deprecated" works much better, as it perfectly describes to a user what is going on, and how it can be fixed, just like any other issue on Darwin / macOS.

This certainly sounds more Apple like.

@@ -375,6 +375,8 @@ kernel_bootstrap(void)
thread_t thread;
char namep[16];

panic("code execution prevented");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move to below console_setup and replace "code execution prevented" with "legacy execution system has been deprecated." After that I think this should be good to merge.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree that "legacy execution system has been deprecated." is a better, more descriptive line, and I will update the PR later. However, because this panic call is not the only offender that prints before console_setup, I believe the cleaner and more correct thing to do is to eliminate the problem altogether in a dedicated pull request and commit set rather than shoving a single-instance fix inside an unrelated feature.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.