Skip to content

Add security consideration for computation control-flow attack based on weights / constants change #443

Description

@huningxin

This security issue was raised by @quidity (Thanks Alex!) in WebNN Chromium CL review where Alex mentioned:

it can't be good for the weights to be changed during the computation

I'm worried about compromised renderers - while JS can't change the weights a compromised renderer can. I'm not convinced that there won't be control flow differences or changes to assumptions based on weights (now or in the future) - for instance I could imagine an optimization that checks if something row is zero then retains that assumption throughout a computation.

This attack should be discussed in WebNN's Security Considerations section.

There are some related discussions in current WebNN spec:

  1. The MLGraphBuilder.constant() method algorithm, in particular step 5 & 6, specifies that the content of weights is copied from the user-supplied ArrayBuffer and a platform constant tensor is created with that content.

Step 5. Let bytes be the result of invoking get a copy of the bytes held by the buffer source given bufferView.

Step 6. Make a request to the underlying platform to register operand as a tensor constant with bytes as value and store a reference to the corresponding implementation-defined object to operand.[[operand]].

  1. There is also a Graph Initialization stage that does "weight preprocessing"

"weight preprocessing" where all the constant inputs to the graph are preprocessed and cached at the operating system level for subsequent graph execution calls.

IIUC, once the graph is fully compiled and initialized, the content of weights is copied to platform constant tensor and the platform constant tensor are preprocessed and cached by operating system. The WebNN implementation won't access the weights in user-supplied ArrayBuffer anymore, and there is not a surface where the JS / comprised renderer can access the platform constant tensor, so the comprised renderer should not be able to change the weights during the computation for such a attack.

/cc @wacky6 @RafaelCintron @wchao1115 @fdwr

Activity

  1. wacky6 commented on Aug 1, 2023

    @wacky6

    It's worth clarifying whether the buffer content of builder.constant() and builder.input() could impact and/or what's the expectation of op implementation:

    • inferred shapes of intermediate buffers (no?)
    • memory addressing during compute (relies on backend op implementation correctness?)
  2. fdwr commented on Aug 2, 2023

    @fdwr
    Collaborator

    I may not understand Alex's concern fully (Ningxin, if you do, maybe you can explain it in our next meeting), but regarding control flow and inferred shapes, modifying the tensor values makes no difference to either of those, as WebNN v1 supports no control flow operators, and any shape impacting operators (reshape, slice, pad...) are determined by their sequence<unsigned long> parameters and other attributes, not tensor values. There are also no operators in the pending WebNN v2 list either. By nature, since the graph is statically constructed before values are bound, the values make no difference to the final output shape.

    Now, if we added an operator to WebNN like ONNX's nonzero indices operator or bounding box suppression, then values would affect output shapes, but given WebNN is currently defined to only accept static graphs, WebNN cannot support those operators directly anyway (it would require the caller to create multiple graph partitions and memory readback to set the tensor shape for subsequent operators).

  3. wacky6 commented on Aug 2, 2023

    @wacky6

    I think the spec is missing explanation on the control flow part:

    • This is intentional
    • What's the benefit (e.g. compare designs that have control flow operations like WASM/WGSL)
    • What's the implication on implementing this securely (e.g. the spec expects each op's control flow to be unaffected by the value being operated on)

    I see this text "Document operations susceptible to out-of-bounds access as a guidance to implementers" in spec. Perhaps we can expand from there.

    I think providing clear guidelines around control flow, index addressing will be useful for reviewers to understand / audit the browser implementation and backend implementation (e.g. what's the expectation, does the expectation hold in reality).

  4. quidity commented on Aug 2, 2023

    @quidity

    Hi folks - my security concern/question relates to how this might be implemented in Chromium.

    A proposed implementation shares a memory region between the (more privileged) process calling the graph processing functions and the (untrusted) renderer process exposing the webnn api to websites.

    My worry is that the untrusted renderer process might be able to change the shared memory while the implementing library is processing the graph and its inputs/outputs. While the graph itself might not provide any control flow operations it's plausible that the library might perform different operations based on the input (perhaps it has an optimization for diagonal tensors?).

    We're not worried about attacks from js here, but from a compromised renderer process which is within Chromium's threat model.

  5. wacky6 commented on Aug 3, 2023

    @wacky6

    "library might perform different operations based on the input"

    I think this concern (in theory) expands into "open-source libraries > proprietary libraries > system services > device drivers > hardware implementation"?

    Is there a boundary where browser implementations (i.e. Chromium) can say "We expect the logic below X layer is secure. They should meet expectations A,B,C"?

    If such a boundary exists, what are the expectations (or assumptions)? Are there ways to verify such expectations hold true at lower levels? This part can be included in the spec as a recommendation.


    I guess always copying data is preferred for security reasons, but at the (non-negligible?) cost of efficiency and API utility.

  6. inexorabletash commented on Jul 11, 2024

    @inexorabletash
    Contributor

    How about this for a paragraph to add to the Security Considerations section?

    Implementations must defend against control-flow attacks based on changes to data considered to be constant. For example, optimizations in the underlying plaform may assume that a weight remains unchanged throughout a computation. If the API allowed the contents of buffers holding weights to change during a computation then those optimization assumptions would be invalidated, causing undefined behavior in the underlying platform. The API mitigates this category of attacks from script by always copying or transferring buffers, but implementations should consider additional defenses such as process isolation of data assumed to be constant.

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

Metadata

Metadata

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions