Add security guidelines for HANDLE on Windows

Bug: 406023316
Change-Id: Idb8910030239bbbd30940e0186696ba47497750b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6444933
Commit-Queue: Alex Gough <ajgo@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1445675}

docs/security/rules.md

@@ -17,6 +17,7 @@ This is a list of the security policies Chromium has published.
 * [Guidelines for URL display](url_display_guidelines/url_display_guidelines.md)
 * [Avoid adding cross-origin full-page overlays](overlay-policy.md)
 * [Security Guidelines for LLMs and other large models in Chrome](llm-security-guidelines.md)
+* [Windows HANDLE handling](windows-handle-security-guidelines.md)
 
 You can also find our position on various matters in the [security FAQ](faq.md):
 for example, on local attackers or on the privilege accorded to enterprise

docs/security/windows-handle-security-guidelines.md

@@ -0,0 +1,81 @@
+# Security guidelines for HANDLEs on Windows
+
+TL;DR;
+
+* Use [ScopedHandle](https://source.chromium.org/chromium/chromium/src/+/main:base/win/scoped_handle.h)
+  whenever possible.
+* If you have to use a raw HANDLE:
+  * Reject invalid values using [base::win::IsPseudoHandle()](https://source.chromium.org/chromium/chromium/src/+/main:base/win/windows_handle_util.h)
+  do not just check against INVALID_HANDLE_VALUE,
+  * Initialize and return nullptr on failure.
+  * Avoid storing pseudo handle values.
+  * Provide GetCurrentProcess() or GetCurrentThread() directly to Windows APIs.
+
+## Discussion
+
+Windows HANDLEs are used to represent objects such as Events, Files, Processes
+and Threads. HANDLEs are typedef’d as a pointer-sized type but Windows only uses
+32 bits (with sign extension) so that 32 and 64 bit processes can interoperate.
+
+Windows HANDLEs are generally returned by Windows APIs that create or open
+objects, and must be closed once the object is no longer in use to conserve
+system resources. HANDLEs to the same object might have different access rights
+(for instance, a process handle might only allow waiting on exit, or might allow
+full memory access). HANDLEs can also be duplicated, both within the process
+that created them (to allow code with different lifetimes to refer to the same
+underlying object), and to other processes.
+
+HANDLE has several quirks. Many Windows APIs take or return pseudo handle
+values, such as the return from ::GetCurrentProcess() or ::GetCurrentThread().
+These pseudo handle values may overlap with return values from some handle
+manipulation functions, and malicious code could send these placeholder values
+from less privileged processes.
+
+In particular, GetCurrentProcess() == INVALID_HANDLE_VALUE. Code must be very
+careful not to use these pseudo handle values accidentally. For instance, code
+might intend to open and send a file handle to a renderer, but if the file
+opening failed it might accidentally provide INVALID_HANDLE_VALUE as the source
+handle to DuplicateHandle(), resulting in a handle to the current process being
+sent to the renderer, which would be a serious security bug.
+
+In general it should not be necessary for Chromium code to directly manipulate
+Windows HANDLEs, and instead code should use abstractions such as [base::File](https://source.chromium.org/chromium/chromium/src/+/main:base/files/file.h).
+Chromium, via mojo, makes it easy to send appropriately wrapped objects to
+children using [mojo_base](https://source.chromium.org/chromium/chromium/src/+/main:mojo/public/mojom/base/) wrappers.
+
+# Guidelines
+
+## Use ScopedHandle whenever possible
+
+In Chromium, HANDLEs should always be owned by a [base::win::ScopedHandle](https://source.chromium.org/chromium/chromium/src/+/main:base/win/scoped_handle.h)
+and their underlying HANDLE should only be accessed when a related Windows API
+is called. This ensures that HANDLEs are not leaked and that ownership of the
+underlying object in Chromium code is clear. ScopedHandle refuses to adopt or
+represent pseudo handle values, and will not return them from its .get()
+accessor.
+
+When duplicating a ScopedHandle’s underlying HANDLE always call
+ScopedHandle::is_valid() before calling DuplicateHandle, and return an empty
+ScopedHandle or a raw nullptr HANDLE if duplication fails.
+
+When adopting a HANDLE value into a base::win::ScopedHandle that another process
+duplicated into the current process (e.g. a log file handle on a command line)
+use [base::win::TakeHandleOfType()](https://source.chromium.org/chromium/chromium/src/+/main:base/win/scoped_handle.h)
+to validate that the HANDLE is not a pseudo handle and points to a valid object
+before adopting it.
+
+## Using HANDLE in Chromium
+
+If you must manipulate HANDLEs directly, the following guidelines apply:
+
+Do not store pseudo handles, and instead simply call ::GetCurrentProcess() when
+you need the pseudo handle value. If you need a real handle to a process (and
+[base::Process](https://source.chromium.org/chromium/chromium/src/+/main:base/process/process.h)
+does not provide what you need), you must duplicate to a real HANDLE before
+adopting it.
+
+Use nullptr to represent uninitialized or invalid values. In code that manages
+its own HANDLEs use nullptr to initialize all HANDLE variables and members and
+return nullptr from any accessor that returns a raw HANDLE while the wrapper
+object is invalid. Use [base::win::IsPseudoHandle()](https://source.chromium.org/chromium/chromium/src/+/main:base/win/windows_handle_util.h) to validate HANDLE values
+provided to constructors (do not just check handle != INVALID_HANDLE_VALUE).