Bug 148423 - [WebKit2] add some protection code at IndexedDB
Summary: [WebKit2] add some protection code at IndexedDB
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jincheol Jo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-25 02:19 PDT by Jincheol Jo
Modified: 2022-07-11 15:04 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.86 KB, patch)
2015-08-25 02:24 PDT, Jincheol Jo
mcatanzaro: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jincheol Jo 2015-08-25 02:19:34 PDT
Metadata pointer seems to need a null protection code.
Its member is accessed in next line.
I guess it can trigger crash.
Comment 1 Jincheol Jo 2015-08-25 02:24:32 PDT
Created attachment 259832 [details]
Patch
Comment 2 Brady Eidson 2015-08-25 21:49:23 PDT
Are you just offering this patch because of code inspection, or do you actually have a reproducible case of the null deref crash?
Comment 3 Jincheol Jo 2015-08-26 02:51:40 PDT
(In reply to comment #2)
> Are you just offering this patch because of code inspection, or do you
> actually have a reproducible case of the null deref crash?

When I blocked all function body code of KeyedEncoder and KeyedDecoder for my test, it triggered crash in that line.
Comment 4 Brady Eidson 2015-08-26 09:08:24 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > Are you just offering this patch because of code inspection, or do you
> > actually have a reproducible case of the null deref crash?
> 
> When I blocked all function body code of KeyedEncoder and KeyedDecoder for
> my test, it triggered crash in that line.

For what test? This patch has no test.

And what do you mean "blocked all function body code of KeyedEncoder and KeyedDecoder"?
Comment 5 Brady Eidson 2015-08-26 09:20:36 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Are you just offering this patch because of code inspection, or do you
> > > actually have a reproducible case of the null deref crash?
> > 
> > When I blocked all function body code of KeyedEncoder and KeyedDecoder for
> > my test, it triggered crash in that line.
> 
> For what test? This patch has no test.

Since I hadn't explicitly stated this, I will now - Patches require tests unless there's a darned good reason why they're untestable.

It sounds like you have a way of triggering the crash, we'd like a test that triggers the crash along with the patch.
Comment 6 Jincheol Jo 2015-08-26 19:31:59 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (In reply to comment #2)
> > > > Are you just offering this patch because of code inspection, or do you
> > > > actually have a reproducible case of the null deref crash?
> > > 
> > > When I blocked all function body code of KeyedEncoder and KeyedDecoder for
> > > my test, it triggered crash in that line.
> > 
> > For what test? This patch has no test.
> 
> Since I hadn't explicitly stated this, I will now - Patches require tests
> unless there's a darned good reason why they're untestable.
> 
> It sounds like you have a way of triggering the crash, we'd like a test that
> triggers the crash along with the patch.

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (In reply to comment #2)
> > > > Are you just offering this patch because of code inspection, or do you
> > > > actually have a reproducible case of the null deref crash?
> > > 
> > > When I blocked all function body code of KeyedEncoder and KeyedDecoder for
> > > my test, it triggered crash in that line.
> > 
> > For what test? This patch has no test.
> 
> Since I hadn't explicitly stated this, I will now - Patches require tests
> unless there's a darned good reason why they're untestable.
> 
> It sounds like you have a way of triggering the crash, we'd like a test that
> triggers the crash along with the patch.
Thanks for quick reply, I will prepare a test.
Comment 7 Michael Catanzaro 2015-12-31 14:42:24 PST
Comment on attachment 259832 [details]
Patch

Removing this from the request queue, pending a testcase.