diff options
| author | Samuel Huang <huangs@chromium.org> | 2018-07-11 19:16:21 +0000 |
|---|---|---|
| committer | Copybara-Service <copybara-worker@google.com> | 2021-07-25 20:30:32 -0700 |
| commit | a09f4e23431ab0718a168d4b37afd9a2923fb1cb (patch) | |
| tree | 39827dd38ad99a9f1f122ec4236489efb3087ab9 /buffer_view_unittest.cc | |
| parent | 620015b71e152d7766651a1b92701b07cce00ce6 (diff) | |
| download | zucchini-a09f4e23431ab0718a168d4b37afd9a2923fb1cb.tar.gz | |
[Zucchini] Fix BufferViewBase::covers_array() to allow 0-sized array at end of buffer.
BufferViewBase::covers_array(offset, num, elt_size) decides whether a
buffer at |offset| can fit an array with |num| elements, each with
|elt_size|. A special case is covers_array(size(), 0, elt_size), i.e.,
can we fit a empty array at end of the buffer?
Previously this was considered to be a pathological case, so the result
is "false". However, recently it's revealed that this causes some valid
DEX files to rejected!
What happens is that ParseAnnotationDirectoryItem() parses data that
look like (in regex) "(AF*M*P*)*", where "AF*M*P*" is a block with
header "A" with counts for structs "F", "M", "P", followed by the
specified number of these structs. The parsing code uses covers_array()
to check for buffer overrun. However, for the case where the last
"AF*M*P*" block has 0 "P" blocks, we'd encounter the special case
covers_array(size(), 0, elt_size), and the resulting "false"
invalidates the DEX file.
The fix is to make the special case return "true". Note that this only
affects DEX (which is currently the only user of covers_array()).
Change-Id: I2939194f7e91739193e1558361aeb9617bf9c023
Reviewed-on: https://chromium-review.googlesource.com/1133688
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574279}
NOKEYCHECK=True
GitOrigin-RevId: 1b1153fc0b354fd73f63c2324753ad79b42f3fc1
Diffstat (limited to 'buffer_view_unittest.cc')
| -rw-r--r-- | buffer_view_unittest.cc | 6 |
1 files changed, 3 insertions, 3 deletions
diff --git a/buffer_view_unittest.cc b/buffer_view_unittest.cc index 1d3ccb8..b048747 100644 --- a/buffer_view_unittest.cc +++ b/buffer_view_unittest.cc @@ -194,10 +194,10 @@ TEST_F(BufferViewTest, CoversArray) { EXPECT_TRUE(view.covers_array(0, 0, bytes_.size())); EXPECT_TRUE(view.covers_array(bytes_.size() - 1, 0, bytes_.size())); - EXPECT_FALSE(view.covers_array(bytes_.size(), 0, bytes_.size())); + EXPECT_TRUE(view.covers_array(bytes_.size(), 0, bytes_.size())); EXPECT_TRUE(view.covers_array(0, 0, 0x10000)); EXPECT_TRUE(view.covers_array(bytes_.size() - 1, 0, 0x10000)); - EXPECT_FALSE(view.covers_array(bytes_.size(), 0, 0x10000)); + EXPECT_TRUE(view.covers_array(bytes_.size(), 0, 0x10000)); EXPECT_FALSE(view.covers_array(0, 1, bytes_.size() + 1)); EXPECT_FALSE(view.covers_array(0, 2, bytes_.size())); @@ -206,7 +206,7 @@ TEST_F(BufferViewTest, CoversArray) { EXPECT_FALSE(view.covers_array(1, bytes_.size(), 1)); EXPECT_FALSE(view.covers_array(bytes_.size(), 1, 1)); - EXPECT_FALSE(view.covers_array(bytes_.size(), 0, 1)); + EXPECT_TRUE(view.covers_array(bytes_.size(), 0, 1)); EXPECT_FALSE(view.covers_array(0, 0x10000, 0x10000)); } |