WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
206572
KeyedDecoderGeneric crashes when it accesses a data with empty string key.
https://bugs.webkit.org/show_bug.cgi?id=206572
Summary
KeyedDecoderGeneric crashes when it accesses a data with empty string key.
Takashi Komori
Reported
2020-01-21 23:28:42 PST
When KeyedDecoderGeneric decodes a record associated with empty string key, it tries to add a record to a HashMap with null string (not empty string) but adding HashMap fails because HashMap can't treat null string as a key. While adding a record with null string key, StringHash::hash() crashes by accessing null pointer.
Attachments
Patch
(3.09 KB, patch)
2020-01-21 23:46 PST
,
Takashi Komori
no flags
Details
Formatted Diff
Diff
Patch
(3.04 KB, patch)
2020-01-23 00:57 PST
,
Takashi Komori
no flags
Details
Formatted Diff
Diff
Patch
(3.21 KB, patch)
2020-01-23 01:41 PST
,
Takashi Komori
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Takashi Komori
Comment 1
2020-01-21 23:46:26 PST
Created
attachment 388399
[details]
Patch
Takashi Komori
Comment 2
2020-01-21 23:51:05 PST
Added test just encodes/decodes a boolean data with empty string key but I only tested on wincairo port.
Fujii Hironori
Comment 3
2020-01-22 00:52:06 PST
How did you find this bug? Is this a real use case? empty string key.
Takashi Komori
Comment 4
2020-01-22 02:37:21 PST
(In reply to Fujii Hironori from
comment #3
)
> How did you find this bug? Is this a real use case? empty string key.
This crash occurred and found when I tried to decode broken encoded data (zero filled from middle) by chance. So I think it doesn't occur in normal scenarios. To avoid decoding collapsed data, I tried to verify encoded data by using Decoder::verifyChecksum (PersistentDecoder.cpp) But it didn't work because we can't verify before all decoding is done. Persistence::Decoder calculates checksum while decoding data.
Fujii Hironori
Comment 5
2020-01-22 03:25:17 PST
String::fromUTF8 will return a null string for invalid UTF-8 sequence. You should check null-string after caling String::fromUTF8. result = String::fromUTF8(buffer.data(), size);
Takashi Komori
Comment 6
2020-01-23 00:57:41 PST
Created
attachment 388524
[details]
Patch
Takashi Komori
Comment 7
2020-01-23 00:59:01 PST
(In reply to Fujii Hironori from
comment #5
)
> String::fromUTF8 will return a null string for invalid UTF-8 sequence. > You should check null-string after caling String::fromUTF8. > > result = String::fromUTF8(buffer.data(), size);
Fixed.
Fujii Hironori
Comment 8
2020-01-23 01:02:11 PST
Comment on
attachment 388524
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=388524&action=review
> Source/WebCore/platform/generic/KeyedDecoderGeneric.cpp:61 > + result = emptyString();
I think if String::fromUTF8 is failed, a null string and false should be returned. Of course, an empty string should be return for size == 0 case. WDTY?
Takashi Komori
Comment 9
2020-01-23 01:41:51 PST
Created
attachment 388525
[details]
Patch
Takashi Komori
Comment 10
2020-01-23 01:44:31 PST
(In reply to Fujii Hironori from
comment #8
)
> Comment on
attachment 388524
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=388524&action=review
> > > Source/WebCore/platform/generic/KeyedDecoderGeneric.cpp:61 > > + result = emptyString(); > > I think if String::fromUTF8 is failed, a null string and false should be > returned. > > Of course, an empty string should be return for size == 0 case. > > WDTY?
I agree. Failing fromUTF8 means decoded string is collapsed. Fixed.
WebKit Commit Bot
Comment 11
2020-01-23 02:58:21 PST
The commit-queue encountered the following flaky tests while processing
attachment 388525
[details]
: editing/spelling/spellcheck-attribute.html
bug 206178
(authors:
g.czajkowski@samsung.com
,
mark.lam@apple.com
, and
rniwa@webkit.org
) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 12
2020-01-23 02:58:53 PST
Comment on
attachment 388525
[details]
Patch Clearing flags on attachment: 388525 Committed
r254971
: <
https://trac.webkit.org/changeset/254971
>
WebKit Commit Bot
Comment 13
2020-01-23 02:58:54 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 14
2020-01-23 02:59:14 PST
<
rdar://problem/58829480
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug