New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix invalid IntPtr == null comparisons, set strict mode for Roslyn #19191

Merged
merged 1 commit into from Jul 30, 2018

Conversation

Projects
None yet
4 participants
@4creators
Collaborator

4creators commented Jul 29, 2018

Issue reported in dotnet/corefx#31456
Solution is to compare always against IntPtr.Zero and use Roslyn
stric mode for reporting warnings for IntPtr == null comparisons

Does not seem to have any security impact

Fix invalid IntPtr == null comparisons, set strict mode for Roslyn
Issue reported in dotnet/corefx#31456
Solution is to compare always against IntPtr.Zero and use Roslyn
stric mode for reporting warnings for IntPtr == null comparisons
@4creators

This comment has been minimized.

Collaborator

4creators commented Jul 29, 2018

@danmosemsft

This comment has been minimized.

Member

danmosemsft commented Jul 29, 2018

@jkotas before I merge, is there any reason we didn't enable this before?

@tannergooding

This comment has been minimized.

Member

tannergooding commented Jul 29, 2018

Cc. @jaredpar

@danmosemsft

This comment has been minimized.

Member

danmosemsft commented Jul 29, 2018

@4creators are there any other different in the IL for the assembly? Although probably @jaredpar can tell us that it only affects diagnostics.

@4creators

This comment has been minimized.

Collaborator

4creators commented Jul 29, 2018

@4creators are there any other different in the IL for the assembly?

@danmosemsft Would need to write some regex to search - will do it tomorrow.

So far in current .NET Core I have not seen any security problems, however, one should check if all strlen implementations used by VM via StubHelpers.strlen fast call are safe. Otherwise, we may have potential nullptr dereference vulnerability in CLR native code or worse.

@jkotas

jkotas approved these changes Jul 30, 2018

@jkotas jkotas merged commit 93cf012 into dotnet:master Jul 30, 2018

27 checks passed

CROSS Check Build finished.
Details
CentOS7.1 x64 Checked Innerloop Build and Test Build finished.
Details
CentOS7.1 x64 Debug Innerloop Build Build finished.
Details
Linux-musl x64 Debug Build Build finished.
Details
OSX10.12 x64 Checked CoreFX Tests Build finished.
Details
OSX10.12 x64 Checked Innerloop Build and Test Build finished.
Details
Tizen armel Cross Checked Innerloop Build and Test Build finished.
Details
Ubuntu arm Cross Checked Innerloop Build and Test Build finished.
Details
Ubuntu arm Cross Checked no_tiered_compilation_innerloop Build and Test Build finished.
Details
Ubuntu arm64 Cross Debug Innerloop Build Build finished.
Details
Ubuntu x64 Checked CoreFX Tests Build finished.
Details
Ubuntu x64 Checked Innerloop Build and Test Build finished.
Details
Ubuntu x64 Checked Innerloop Build and Test (Jit - TieredCompilation=0) Build finished.
Details
Ubuntu x64 Formatting Build finished.
Details
WIP ready for review
Details
Windows_NT x64 Checked CoreFX Tests Build finished.
Details
Windows_NT x64 Checked Innerloop Build and Test Build finished.
Details
Windows_NT x64 Checked Innerloop Build and Test (Jit - TieredCompilation=0) Build finished.
Details
Windows_NT x64 Formatting Build finished.
Details
Windows_NT x64 full_opt ryujit CoreCLR Perf Tests Correctness Build finished.
Details
Windows_NT x64 min_opt ryujit CoreCLR Perf Tests Correctness Build finished.
Details
Windows_NT x86 Checked Innerloop Build and Test Build finished.
Details
Windows_NT x86 Checked Innerloop Build and Test (Jit - TieredCompilation=0) Build finished.
Details
Windows_NT x86 Release Innerloop Build and Test Build finished.
Details
Windows_NT x86 full_opt ryujit CoreCLR Perf Tests Correctness Build finished.
Details
Windows_NT x86 min_opt ryujit CoreCLR Perf Tests Correctness Build finished.
Details
license/cla All CLA requirements met.
Details

@4creators 4creators deleted the dotnetrt:fix_intptr_null branch Jul 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment