New issue
Advanced search Search tips

Issue 912304 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 5
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Manual Channel Message deserialization can be incorrect with bad inputs

Project Member Reported by rockot@google.com, Dec 5

Issue description

Channel::Message::Deserailize has a DCHECK which is actually true if the input message is of type NORMAL or NORMAL_LEGACY, but local fuzzing reveals that we can trip this DCHECK because we don't fully validate the input message type.

This does not appear to be a security issue since it doesn't result in any bad memory accesses, and the bad message would fails validation gracefully, further along its processing. But it does mean the DCHECK is currently incorrect.
 
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 5

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

commit a1238b1b85d66e4df89305261f3e817aff3b03fb
Author: Ken Rockot <rockot@google.com>
Date: Wed Dec 05 22:20:45 2018

[mojo-core] Add Channel Message type validation

This effectively corrects a DCHECK in Message deserialization code by
aligning Channel::Message::Deserialize and the Message constructor on
their expectation of header sizes for different message types.

Deserialize was assuming that any type other than NORMAL must have a
legacy header, while the constructor assumes that any type other than
NORMAL_LEGACY must have a non-legacy header. In normal circumstances
this doesn't matter because Deserialize is only ever intentionally used
on NORMAL or NORMAL_LEGACY messages, but in the context of a fuzzer or a
misbehaving sender, it is possible for this mismatch to violate a
DCHECK (see the bug).

This changes Deserialize to match the constructor, so anything other
than NORMAL_LEGACY is treated as having a non-legacy header.

Bug:  912304 
Change-Id: If5f35a7723004d79447831eeb80c3ed14e5977a4
Reviewed-on: https://chromium-review.googlesource.com/c/1363806
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#614128}
[modify] https://crrev.com/a1238b1b85d66e4df89305261f3e817aff3b03fb/mojo/core/channel.cc

Status: Fixed (was: Started)

Sign in to add a comment