New issue
Advanced search Search tips

Issue 640298 link

Starred by 2 users
Status: Available
Owner: ----
Cc:
rockot@google.com
sa...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Mojo JS lite bindings: Enforce max recursion depth when decoding structures

Project Member Reported by yzshen@chromium.org, Aug 23 2016 
It is legitimate to have such a recursive mojo definition:

struct Foo {
  Foo nested_foo;
};

If a malicious sender constructs a message with an very deeply-nested Foo object, our validation code may cause the call stack to overflow.

One solution is to have a depth counter in ValidationContext and issue validation error when the counter exceeds a certain number.

Comment 1 by yzshen@chromium.org, Aug 23 2016

Components: Internals>Mojo

Comment 2 by tibell@chromium.org, Sep 5 2016

Owner: tibell@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 8 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe0adee770041fc6ab06582579c16e165346f22a

commit fe0adee770041fc6ab06582579c16e165346f22a
Author: tibell <tibell@chromium.org>
Date: Thu Sep 08 01:35:47 2016

Limit Mojo messages recursion depth

This prevents a malicious sender from sending deeply nested messages
that cause the receiver to blow the stack during message validation.

BUG=640298

Review-Url: https://codereview.chromium.org/2312813002
Cr-Commit-Position: refs/heads/master@{#417151}

[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTest.java
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/cpp/bindings/lib/validation_context.cc
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/cpp/bindings/lib/validation_context.h
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/cpp/bindings/lib/validation_errors.cc
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/cpp/bindings/lib/validation_errors.h
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/cpp/bindings/lib/validation_util.h
[add] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/interfaces/bindings/tests/data/validation/conformance_mthd19_good.data
[add] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/interfaces/bindings/tests/data/validation/conformance_mthd19_good.expected
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/interfaces/bindings/tests/validation_test_interfaces.mojom
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/js/validation_unittests.js

Comment 4 by tibell@chromium.org, Sep 8 2016 

C++ is now done. We still need to enforce the recursion depth limit in Java and JS.
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 8 2016

Labels: merge-merged-2854
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe0adee770041fc6ab06582579c16e165346f22a

commit fe0adee770041fc6ab06582579c16e165346f22a
Author: tibell <tibell@chromium.org>
Date: Thu Sep 08 01:35:47 2016

Limit Mojo messages recursion depth

This prevents a malicious sender from sending deeply nested messages
that cause the receiver to blow the stack during message validation.

BUG=640298

Review-Url: https://codereview.chromium.org/2312813002
Cr-Commit-Position: refs/heads/master@{#417151}

[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/android/javatests/src/org/chromium/mojo/bindings/ValidationTest.java
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/cpp/bindings/lib/validation_context.cc
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/cpp/bindings/lib/validation_context.h
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/cpp/bindings/lib/validation_errors.cc
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/cpp/bindings/lib/validation_errors.h
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/cpp/bindings/lib/validation_util.h
[add] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/interfaces/bindings/tests/data/validation/conformance_mthd19_good.data
[add] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/interfaces/bindings/tests/data/validation/conformance_mthd19_good.expected
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/interfaces/bindings/tests/validation_test_interfaces.mojom
[modify] https://crrev.com/fe0adee770041fc6ab06582579c16e165346f22a/mojo/public/js/validation_unittests.js

Comment 7 by yzshen@chromium.org, Jan 9 2017

Components: -Internals>Mojo Internals>Mojo>Bindings

Comment 8 by tibell@chromium.org, Jun 6 2017

Owner: ----
Status: Available (was: Untriaged)
Someone should do the same for JS, otherwise we're done.
Project Member

Comment 9 by sheriffbot@chromium.org, Jun 6 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 10 by roc...@chromium.org, Oct 17

Cc: -roc...@chromium.org rockot@google.com

Comment 11 by rockot@google.com, Nov 15

Cc: -yzshen@chromium.org -tibell@chromium.org
Status: Available (was: Untriaged)
Summary: Mojo JS lite bindings: Enforce max recursion depth when decoding structures (was: Mojo message validation: consider introducing max-depth restriction to avoid stack overflow)
Triage, renaming to reflect the remaining work.
►

Sign in to add a comment