c# security review checklist

General Code Review Guidelines

Check	Description
	Potential threats are clearly documented. (Threats are dependent upon the specific scenario and assembly type.)
	Code is developed based on .NET framework coding guidelines and secure coding guidelines at http://msdn.microsoft.com/en-us/library/czefa0ke(VS.71).aspx.
	The FXCop analysis tool is run on assemblies and security warnings are addressed.

Managed Code Review Guidelines

Assembly-Level Checks

Check	Description
	Assemblies have a strong name. (Dynamically generated ASP.NET Web page assemblies cannot currently have a strong name.)
	You have considered delay signing as a way to protect and restrict the private key that is used in the strong name and signing process.
	Assemblies include declarative security attributes (with SecurityAction.RequestMinimum) to specify minimum permission requirements.
	Highly privileged assemblies are separated from lower privileged assemblies. If the assembly is to be used in a partial-trust environment (for example, it is called from a partial-trust Web application), then privileged code is sandboxed in a separate assembly.

Class-Level Checks

Check	Description
	Class and member visibility is restricted. The most restrictive access modifier is used (private where possible).
	Non-base classes are sealed if they contain security secrets (like passwords) accessible through protected APIs or if they contain many virtual members that cannot be sealed and the type is not really designed for third-party extensibility.
	Input from outside the current trust boundary is validated. Input data is constrained and validated for type, length, format, and range.
	Code implements declarative checks where virtual internal methods are used.
	Access to public classes and methods are restricted with principal permission demands (where appropriate).
	Fields are private. When necessary, field values are exposed by using read/write or read-only public properties.
	Read-only properties are used where possible.
	Types returned from methods that are not designed to be created independently contain private default constructors.
	Unsealed public types do not have internal virtual members.
	Use of event handlers is thoroughly reviewed.
	Static constructors are private.

Cryptography

Check	Description
	Code uses platform-provided cryptography and does not use custom implementations.
	Random keys are generated by using RNGCryptoServiceProvider (and not the Random class).
	PasswordDeriveBytes is used for password-based encryption.
	DPAPI is used to encrypt configuration secrets to avoid the key management issue.
	The appropriate key sizes are used for the chosen algorithm, or if they are not, the reasons are identified and understood.
	Keys are not held in code.
	Access to persisted keys is restricted.
	Keys are cycled periodically.
	Exported private keys are protected.

Secrets

Check	Description
	Secrets are not hard coded.
	Plain text secrets are not stored in configuration files.
	Plain text secrets are not stored in memory for extended periods of time.

Exception Management

Check	Description
	Code uses exception handling. You catch only the exceptions that you know about.
	Exception details are logged on the server to assist in diagnosing problems.
	The information that is returned to the end user is limited and safe.
	Code that uses exception filters is not sensitive to filter execution sequence (filter runs before finally block).
	Code fails early to avoid unnecessary processing that consumes resources.
	Exception conditions do not allow a user to bypass security checks to run privileged code.

Delegates

Check	Description
	Delegates are not accepted from untrusted sources.
	If code does accept a delegate from untrusted code, it constrains the delegate before calling it by using security permissions with SecurityAction.PermitOnly.
	Permissions are not asserted before calling a delegate.

Serialization

Check	Description
	Serialization is restricted to privileged code.
	Sensitive data is not serialized.
	Field data from serialized data streams is validated.
	ISerializable.GetObjectData implementation is protected with an identity permission demand in scenarios where you want to restrict which code can serialize the object.

Threading

Check	Description
	Results of security checks are not cached.
	Impersonation tokens are considered when new threads are created (any existing thread token is not passed to the new thread).
	Threads are synchronized in static class constructors for multithreaded application code.
	Object implementation code is designed and built to be thread safe.
	Threads are synchronized in static class constructors.

Reflection

Check	Description
	Caller cannot influence dynamically generated code (for example, by passing assembly and type names as input arguments).
	Code demands permission for user authorization where assemblies are loaded dynamically.

Unmanaged Code Access

Check	Description
	Input and output strings that are passed between managed and unmanaged code are constrained and validated.
	Array bounds are checked.
	File path lengths are checked and do not exceed MAX_PATH.
	Unmanaged code is compiled with the /GS switch.
	Use of "dangerous" APIs by unmanaged code is closely inspected. These include LogonUser, RevertToSelf, CreateThread, Network APIs, and Sockets APIs.
	Naming conventions (safe, native, unsafe) are applied to unmanaged APIs.
	Assemblies that call unmanaged code specify unmanaged permission requirements using declarative security (SecurityAction.RequestMinimum).
	Unmanaged API calls are sandboxed and isolated in a wrapper assembly.
	Use of SuppressUnmanagedCodeSecurityAttribute is thoroughly reviewed and additional security checks are implemented.
	Types are not annotated with SuppressUnmanagedCodeSecurityAttribute. (This attribute is used on specific P/Invoke method declarations instead.)
	Calling code is appropriately authorized using a full stack walk Demand (using either a .NET Framework permission or custom permission).
	Unmanaged types or handles are never exposed to partially trusted code.
	Pointers are private fields.
	Methods that use IntPtr fields in a type that has a finalizer call GC.KeepAlive(object).

Resource Access Considerations

File I/O

Check	Description
	No security decisions are made based on filenames.
	Input file paths and file names are well formed.
	Environment variables are not used to construct file paths.
	File access is constrained to the context of the application (by using a restricted FileIOPermission).
	Assembly file I/O requirements are specified using declarative security attributes (with SecurityAction.RequestMinimum).

Event Log

Check	Description
	Event log access code is constrained using EventLogPermission. This particularly applies if your event logging code could be called by untrusted callers.
	Event sources are created at installation time. If unable to create event sources at installation time, the administrator manually creates a new event source entry in the registry. The account used to run the code that writes to the event log is not allowed to create new event sources by configuring ACL in the registry.
	Security-sensitive data, such as passwords, is not written to the event log.

Registry

Check	Description
	Sensitive data, such as database connection strings or credentials, is encrypted prior to storage in the registry.
	Keys are restricted. If a key beneath HKEY_CURRENT_MACHINE is used, the key is configured with a restricted ACL. Alternatively, HKEY_CURRENT_USER is used.
	Registry access is constrained by using RegistryPermission. This applies especially if your registry access code could be called by untrusted callers.

Environment Variables

Check	Description
	Code that accesses environment variables is restricted with EnvironmentPermission. This applies especially if your code can be called by untrusted code.
	Environment permission requirements are declared by using declarative security attributes with SecurityAction.RequestMinimum.

Code Access Security Considerations

If an entry is preceded by a star (*), it indicates that the checks are performed by the FXCop analysis tool. For more information about FXCop security checks, see http://code.msdn.microsoft.com/GotDotNet.aspx.

Check	Description
	Assemblies marked with AllowPartiallyTrustedCallersAttribute (APTCA) do not expose objects from non-APTCA assemblies.
	Code that only supports full-trust callers is strong named or explicitly demands the full-trust permission set.
	All uses of Assert are thoroughly reviewed.
	All calls to Assert are matched with a corresponding call to RevertAssert.
	*The Assert window is as small as possible.
	*Asserts are proceeded with a full permission demand.
	*Use of Deny or PermitOnly is thoroughly reviewed.
	All uses of LinkDemand are thoroughly reviewed. (Why is a LinkDemand and not a full Demand used?)
	LinkDemands within Interface declarations are matched by LinkDemands on the method implementation.
	*Unsecured members do not call members protected by a LinkDemand.
	Permissions are not demanded for resources accessed through the .NET Framework classes.
	Access to custom resources (through unmanaged code) is protected with custom code access permissions.
	Access to cached data is protected with appropriate permission demands.
	If LinkDemands are used on structures, the structures contain explicitly defined constructors.
	*Methods that override other methods that are protected with LinkDemands also issue the same LinkDemand.
	*LinkDemands on types are not used to protect access to fields inside those types.
	*Partially trusted methods call only other partially trusted methods.
	*Partially trusted types extend only other partially trusted types.
	*Members that call late bound members have declarative security checks.
	*Method-level declarative security does not mistakenly override class-level security checks.
	Use of the following "potentially dangerous" permissions is thoroughly reviewed: SecurityPermission Unmanaged Code SkipVerification ControlEvidence ControlPolicy SerializationFormatter ControlPrincipal ControlThread ReflectionPermission MemberAccess
	Code identity permission demands are used to authorize calling code in scenarios where you know in advance the range of possible callers (for example, you want to limit calling code to a specific application).
	Permission demands of the .NET Framework are not duplicated.