New issue
Advanced search Search tips

Issue 879237 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 1
Type: Task



Sign in to add a comment

Audit and enhance the CBOR parser

Project Member Reported by palmer@chromium.org, Aug 30

Issue description

Examine and, if necessary, harden components/cbor for memory safety, optional feature non-support (or at least caller-configured), and semantic correctness.

Also, extend the API so that callers can set a maximum memory consumption limit. There is a potentially large multiplier from input size to memory consumed, so we don't want calling processes to get bombed.

Where and if possible, ensure that the API is easy to call correctly and hard to call incorrectly.
 
Labels: -Type-Bug Type-Task
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 14

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

commit be2d8dcbe045805779a972a32bc1433dd782b01a
Author: Chris Palmer <palmer@chromium.org>
Date: Fri Sep 14 00:31:42 2018

Document `CBORReader`'s limitations and their reasons.

Also resolve an API inconsistency: use Optional in 1 last place.

Bug: 879237
Change-Id: I383cce38e47f06cedff99c05c3b2c1e1c67601f7
Reviewed-on: https://chromium-review.googlesource.com/1222916
Commit-Queue: Chris Palmer <palmer@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591228}
[modify] https://crrev.com/be2d8dcbe045805779a972a32bc1433dd782b01a/components/cbor/BUILD.gn
[rename] https://crrev.com/be2d8dcbe045805779a972a32bc1433dd782b01a/components/cbor/cbor_constants.h
[modify] https://crrev.com/be2d8dcbe045805779a972a32bc1433dd782b01a/components/cbor/cbor_reader.cc
[modify] https://crrev.com/be2d8dcbe045805779a972a32bc1433dd782b01a/components/cbor/cbor_reader.h
[modify] https://crrev.com/be2d8dcbe045805779a972a32bc1433dd782b01a/components/cbor/cbor_reader_unittest.cc
[modify] https://crrev.com/be2d8dcbe045805779a972a32bc1433dd782b01a/components/cbor/cbor_values.cc
[modify] https://crrev.com/be2d8dcbe045805779a972a32bc1433dd782b01a/components/cbor/cbor_values.h
[modify] https://crrev.com/be2d8dcbe045805779a972a32bc1433dd782b01a/components/cbor/cbor_writer.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 17

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

commit 62403b01f1a3edbe0676d4b1f44fa5cb33013006
Author: Chris Palmer <palmer@chromium.org>
Date: Mon Sep 17 22:44:05 2018

Don't `NOTREACHED` when handling malformed CBOR inputs.

Just return `nullopt` and (crucially) set the error code without crashing.

Bug: 879237, 884382 
Change-Id: I899ceb45648e130d5352a0a487c2d244f41a118d
Reviewed-on: https://chromium-review.googlesource.com/1228938
Commit-Queue: Chris Palmer <palmer@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591854}
[modify] https://crrev.com/62403b01f1a3edbe0676d4b1f44fa5cb33013006/components/cbor/cbor_reader.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 19

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

commit 867100ad676118c091dbf583b3d2859bd1a186b7
Author: Chris Palmer <palmer@chromium.org>
Date: Fri Oct 19 18:58:46 2018

Worry over some nits in the CBOR reader.

Make function names the same kinds of phrases (predicates: "IsFoo" and
"HasFoo"); consistent use of curly braces to avoid visual confusion; use
positive conditions (`DCHECK(success, ...)`).

Bug: 879237
Change-Id: I0157f5676bddd7f8a806b5ece9e95792fbbe3ed0
Reviewed-on: https://chromium-review.googlesource.com/c/1289957
Commit-Queue: Chris Palmer <palmer@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601246}
[modify] https://crrev.com/867100ad676118c091dbf583b3d2859bd1a186b7/components/cbor/cbor_reader.cc
[modify] https://crrev.com/867100ad676118c091dbf583b3d2859bd1a186b7/components/cbor/cbor_reader.h

Sign in to add a comment