zyantific / zydis-rs

Zydis Rust Bindings
MIT License
83 stars 14 forks source link

Add windows-kernel flag #39

Closed carlos-al closed 4 months ago

carlos-al commented 4 months ago

To resolve Issue https://github.com/zyantific/zydis-rs/issues/38

athre0z commented 4 months ago

Hmmm. As demonstrated by the CI, the drawback of making this a feature flag is that building with --all-features will no longer work. Perhaps instead of linking the overflow library, we can instruct the C compiler to not build with stack protectors. That'd not break anything even if it is enabled in other builds and should solve the issue as well.

Out of curiosity: what target tuple are you using for the Windows kernel builds?

For MSVC it should probably be this flag: /GS-. For GCC/LLVM, it'd be -fno-stack-protector.

carlos-al commented 4 months ago

Indeed it breaks them, excuse not noting this before.

My target is x86_64-pc-windows-msvc, I'm building with #https://github.com/carlos-al/windows-kernel-rs. This fix could be moved to windows-kernel-sys/build.rs too, and it will work the same. KO when the line is not present, OK when it is.

As for the /GS- flag, I've tried setting it but unfortunately wouldn't make a difference. I've been trying to do it this way for a couple hours to no avail. Modifying both .cargo/config file to include "-C", "link-arg=/GS-" (also as pre-link-arg in case), printing it in build.rs as println!("cargo:rustc-link-arg=/GS-"); both within windows-kernel-sys/build.rs and this project's, but still KO, same imports as noted on the issue. Have retested all these combinations just now after your comment.

Feel free to propose any other modifications and I'll test them.

Note that there are a couple extra ntoskrnl.exe imports too when kernel32.dll is imported.

carlos-al commented 4 months ago

passing it directly to rustc is not recognised, neither as a -C parameter nor -Z one, and this should be a rustc parameter only as per the VS solution. Changing the flag under "code generation" on VS to be /GS- and removing the reference to BufferOverflowK works too. Doesn't seem rustc supports this when using msvc, just with LLVM https://github.com/rust-lang/rust/pull/84197

athre0z commented 4 months ago

I think the issue is that this is a codegen argument, not a linker flag. It's the codegen / compiler that emits the stack cookie checks. Further, we want to pass it to the C compiler, not rustc. Can you see whether the following works:

diff --git a/Cargo.toml b/Cargo.toml
index 6084447..9abec25 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -30,6 +30,7 @@ formatter = ["alloc", "full-decoder"]
 encoder = ["alloc", "full-decoder"]
 serialization = ["serde", "bitflags/serde"]
 nolibc = []
+no-stack-protector = []

 [[example]]
 name = "pattern"
diff --git a/build.rs b/build.rs
index 707439c..95b4470 100644
--- a/build.rs
+++ b/build.rs
@@ -35,11 +35,18 @@ fn build_library() {
         bool2cmake(env::var("CARGO_FEATURE_NOLIBC").is_ok()),
     );

-    let dst = config.build();
-
     let target = env::var("TARGET").unwrap_or("(unknown)".to_string());
     let is_msvc = target.ends_with("windows-msvc");

+    if env::var("CARGO_FEATURE_NO_STACK_PROTECTOR").is_ok() {
+        if is_msvc {
+            config.cflag("/GS-");
+        } else {
+            config.cflag("-fno-stack-protector");
+        }
+    }
+
+    let dst = config.build();
     let relative_build_dir = if is_msvc { config.get_profile() } else { "" };

     println!(

I also pushed this to branch no-stack-prot.

carlos-al commented 4 months ago

It does work as intended. Thanks for the prompt action.

athre0z commented 4 months ago

Cool, thanks for reporting and testing this! :) I merged the branch into master, so let's close this here.