Skip to content
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

A conformance suite with disallowed intra-op examples would be helpful for hardening. #244

Open
quidity opened this issue Dec 22, 2021 · 6 comments
Labels
security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response. testing

Comments

@quidity
Copy link

quidity commented Dec 22, 2021

Much of the security of an implementation of a spec will revolve around rejecting badly formed models, and rejecting changes to tensors that might result in out of bounds accesses.

A suite of failure cases would be helpful to ensure invalid input is properly rejected, and will form useful input to a fuzzer corpus.

(very possible this lives elsewhere!)

@anssiko anssiko added the security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response. label Dec 22, 2021
@anssiko
Copy link
Member

anssiko commented Jan 31, 2022

The group discussed this issue and it was proposed to formalize these failure cases in the test suite that is under development.

@huningxin
Copy link
Contributor

Thanks @quidity .

A suite of failure cases would be helpful to ensure invalid input is properly rejected, and will form useful input to a fuzzer corpus.

+1.

The interfaces interact with user-supplied buffers including MLGraphBuilder::constant, MLGraph::compute. Besides the invalid input buffers, for MLGraph::compute, the invalid output buffers probably also need to be considered. The implementation should reject producing the results into the invalid user-supplied output buffers (e.g., whose size is smaller than expected for output operand) and avoid the out of bounds accesses.

@anssiko
Copy link
Member

anssiko commented Feb 11, 2022

@huningxin if you think you have enough information, feel free to work with @BruceDai to prototype test cases for invalid input and invalid user-supplied output buffers. Any findings or blockers to be documented in this issue.

@quidity please chime in here if you think our plan needs any course correction, or if you have any additional feedback.

@anssiko
Copy link
Member

anssiko commented Aug 10, 2022

@BruceDai please chime in on this issue if you need more information or guidance from the group on how to add the proposed failure cases into the test suite.

@huningxin
Copy link
Contributor

As discussed in Chromium CL-5017758 review for gather op, there is a request to create conformance tests which explicitly perform out-of-bounds access and validates the result conforms to the spec text.

/cc @quidity @RafaelCintron @BruceDai @fdwr

@fdwr
Copy link
Collaborator

fdwr commented Nov 23, 2023

create conformance tests which explicitly perform out-of-bounds access

I agree with this, to ensure no crashes/errors.

validates the result conforms to the spec text

Not sure about this though, if we're expecting the output tensor to exactly match values, as different implementations will have different behaviors for this garbage-in-garbage-out case. Some I can fathom include:

  • Clamp all buffer offset reads to the end of the linear buffer (a simple security measure - DML does this).
  • Clamp all reads to the last element of the axis or 0th element of the axis.
  • Read the 0th element of the axis.
  • Return a special out-of-bounds value (e.g. 0, -1...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response. testing
Projects
None yet
Development

No branches or pull requests

5 participants