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

Latest commit

 

History

History
150 lines (120 loc) · 8.46 KB

safehtml-unchecked.md

File metadata and controls

150 lines (120 loc) · 8.46 KB

Guidelines for Use of Unchecked Conversions of Security Contract Types

Audience note: This document is for developers who think they need to use unchecked conversions, and for security engineers who are reviewing new uses of such conversions.

Overview

Security contract types, such as SafeHtml, let us secure APIs against common vulnerabilities such as XSS and SQL injection. When used correctly, they help Google's security team make sure our applications work on behalf of our users, or at worst, fail safe; but misuse makes it next to impossible for reviewers to check program safety.

The APIs and their related security contract types are designed such that in a typical application, the vast majority of instances of such types can be created using inherently safe APIs (uses of which do not need to be manually security reviewed). For example, F1 uses a SQL Builder interface to create SQL queries that are guaranteed to not be subject to SQL injection vulnerabilities. Similarly, safe HTML types come with various inherently safe builders and factory methods.

There are however scenarios that cannot be expressed using inherently safe APIs, such as available builders of security contract types. To accommodate such use cases, the inherently safe API is accompanied by an API that consumes an arbitrary string without any validation; typically this is embodied as a factory method that performs an unchecked conversion from an arbitrary plain string to an instance of the desired security contract type.

Obviously, such unchecked conversions must only be used if it can be established through careful security code review that for all possible program states, the value supplied to the conversion does in fact satisfy the type contract, or it is otherwise known from context that this use cannot result in security vulnerabilities.

The primary goal of our efforts to create inherently safe APIs is to reduce the amount of code that could potentially harbor security bugs and hence needs manual security review. It also allows us to perform reviews of code that does need manual review with a high degree of confidence. With that in mind:

  • Unchecked conversions should be used rarely, and only if strictly necessary; that is, if inherently safe APIs cannot be used.
  • Use of unchecked conversions should be structured such that it is readily apparent to a security reviewer that the use is indeed safe, and such that it is unlikely that future code modifications result in later introduction of security bugs.

The remainder of this document presents guidelines for appropriate use of unchecked conversions, along with specific examples.

Guidelines

Generally, we BUILD visibility-restrict access to unchecked conversion APIs, and require review and approval before new call sites are allowlisted. This review ensures adherence to these guidelines, and establishes correctness from a security perspective.

Use unchecked conversions only if strictly necessary

To make manual review tractable, the number of call sites of unchecked conversions (each of which needs manual code review) must be kept as low as possible. Therefore, unchecked conversions must not be used if the use case can be expressed in terms of an available inherently safe API.

Use unchecked conversions only within inherently safe APIs

Unchecked conversions should be used only within self-contained, special-purpose libraries or packages that themselves expose an inherently safe API:

  • It must be possible to ascertain that an unchecked conversion used in such a library/package is guaranteed to produce type-contract-compliant values solely based on inspection of the library (and in some cases code the library depends on). The safety of such unchecked conversions must not hinge on code that depends on the library (i.e., the library's clients' code). This guideline allows for one-time review of the code surrounding the unchecked conversion, and avoids the need for (unscalable and difficult to track/ensure) manual code review of all callers of the library.

  • Unchecked conversions must only be used in libraries/packages dedicated to a specific purpose. In particular, they must not be used throughout application code. This guideline ensures that uses of unchecked conversions are limited as much as possible. In our codebase, unchecked conversions are generally provided by separate, BUILD visibility-restricted targets. The targets allowlist is intended to be kept as narrow as possible to discourage introduction of inappropriate uses of unchecked conversions.

  • Unchecked conversions may also be used in code that is generated by a security-reviewed code generator designed to only generate code that upholds type contracts. Generated code should use a special-purpose conversion function separate from unchecked conversions intended to be used in hand-written code.

Unchecked conversions in legacy code and code under refactoring

During ongoing refactorings to adopt use of security contract types in an existing codebase, it is often convenient or necessary to use unchecked conversions. Special purpose "legacy conversions" typically exist for this purpose; e.g. c.g.common.html.types.LegacyConversions and goog.html.legacyconversions. These are used to distinguish conversions which are guaranteed to produce contract-compliant instances of a security contract type - regular unchecked conversions which follow this document's guidelines - from those which don't. Legacy conversions mark code which still poses security risk.

For example, if a web frontend's HTML rendering is converted to the use of a strict autoescaping template system, code that supplies HTML markup to the template system must be changed to provide it in the form of a security contract type, such as c.g.common.html.types.SafeHtml for jslayout, or SanitizedContent for Closure Templates.

Users outside Hardening are heavily discouraged from introducing any new legacy conversion, outside of specialized cases such as:

  • Migration of existing code to a new language, such as JavaScript-to-TypeScript.
  • LSC refactorings that introduce a requirement for safehtml in existing APIs.
  • Eliminating a legacy conversion in a library, by pushing it down to its users.
  • Eliminating a legacy conversion in a client library, by pushing it up to the server code.

Other cases may be the result of copying another part of the codebase, and must be replaced either by a safe API or, if necessary, an unchecked conversion.

In tests, use a test-only conversion

Unchecked conversions may be used in tests where necessary or convenient (since this cannot result in vulnerabilities in production code). However, to distinguish test and non-test use, we typically provide a for-test-only unchecked conversion in a testonly=1 target, e.g., c.g.c.html.types.testing.HtmlConversions.newSafeHtmlForTest and goog.html.testing.newSafeHtmlForTest.

Note that even testonly unchecked conversions should only be used if an inherently safe API is not convenient. This is because they can generate values that non-test APIs would not, and introduce skews between tests and production.