Bug 178598

Summary:

ArrayBufferView Should still give the array buffer even if it's neutered.

Product:

WebKit

Reporter:

Keith Miller <keith_miller>

Component:

JavaScriptCore

Assignee:

Nobody <webkit-unassigned>

Status:

NEW ---

Severity:

Normal

CC:

achristensen, bfulgham, ews-watchlist, fpizlo, jfbastien, mark.lam, mjs, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

Other

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Description Keith Miller 2017-10-20 12:34:47 PDT

...

Comment 1 Radar WebKit Bug Importer 2017-10-20 12:35:07 PDT

<rdar://problem/35100027>

Comment 2 Radar WebKit Bug Importer 2017-10-20 12:35:27 PDT

<rdar://problem/35100031>

Comment 3 Keith Miller 2017-10-20 12:35:52 PDT

<rdar://problem/34005500>

Comment 4 Keith Miller 2017-10-20 12:39:51 PDT

Created attachment 324428 [details]
Patch

Comment 5 Keith Miller 2017-10-20 12:41:22 PDT

Created attachment 324429 [details]
Patch

Comment 6 Saam Barati 2017-10-20 12:46:32 PDT

Comment on attachment 324429 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=324429&action=review

> Source/JavaScriptCore/ChangeLog:7
> +

Can you explain what's going on? It's not obvious from anything that there isn't a bad caller here. Why is it the callee's responsibility. Etc

Comment 7 Maciej Stachowiak 2020-05-30 20:00:04 PDT

Is this old patch still relevant?

Comment 8 Maciej Stachowiak 2020-06-03 14:50:28 PDT

Per discussion with Keith, this does not appear to be actually a security bug.

Comment 9 Maciej Stachowiak 2020-06-04 02:40:43 PDT

Comment on attachment 324429 [details]
Patch

Looks like this still applies and passes tests. Presumably the purpose of this is to prevent a crash or affect some observable behavior, but there's no test case. This also makes it hard to determine why this change is necessary.

Comment 10 Yusuke Suzuki 2020-06-13 10:18:18 PDT

Comment on attachment 324429 [details]
Patch

I think we need to check the existing users of `possiblySharedBuffer()` carefully. I think there are a lot of users which expect `nullptr` as neutered status. But they are now wrong. So we should fix them too.
Another question is, if neutering does not make it nullptr, can we change this signature to `Ref<ArrayBuffer> possiblySharedBuffer() const` instead? If we cannot do that, when do we get nullptr?

Comment 11 Alex Christensen 2021-11-01 11:59:50 PDT

Comment on attachment 324429 [details]
Patch

This has been requesting review for more than one year.  If this is still needed, please rebase and re-request review.