New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 642505 link

Starred by 1 user
Status: WontFix
Owner: ----
Closed: Aug 2016
Cc:
█ roc...@chromium.org
█ yzshen@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Stubs don't validate non-nullable interface types on deserialization

Project Member Reported by roc...@chromium.org, Aug 30 2016 
I may have missed something, but AFACIT we only do a sender-side DCHECK when sending a null interface ptr/request for a non-nullable field. We should validate on the receiving side as well.

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

I thought we did. (And I also took a quick look at the code.)
In debug mode, the sender will DCHECK which terminate the thread so the message is not sent. But in release build, the receiver should reject the message.

Is that a struct field?

Comment 2 by roc...@chromium.org, Aug 30 2016 

Yeah the sender does DCHECK as intended.

And yeah this is non-nullable InterfaceRequest (any interface kind
actually) as a struct field, including message param structs. We just
TakeFoo() the value, and it doesn't look like we ever check for null if the
field is not nullable.

Comment 3 by roc...@chromium.org, Aug 30 2016 

I think we just want to add a check here (https://cs.chromium.org/chromium/src/mojo/public/tools/bindings/generators/cpp_templates/struct_macros.tmpl?rcl=0&l=154) if the field is not nullable?

Comment 4 by yzshen@chromium.org, Aug 30 2016 

The check happens during validation, instead of deserialization.
It is here:
https://codesearch.chromium.org/chromium/src/mojo/public/tools/bindings/generators/cpp_templates/validation_macros.tmpl?rcl=0&l=50

Comment 5 by roc...@chromium.org, Aug 30 2016

Status: WontFix (was: Available)
Ah! Sorry for the noise and thanks for looking. Closing this.
►

Sign in to add a comment