Closed Bug 1335989 Opened 7 years ago Closed 7 years ago

Crashes @ mozilla::ipc::ProcessLink::SendMessageW | IPC_Message_Name=PWyciwygChannel::Msg_WriteToCacheEntry

Categories

(Core :: IPC, defect)

Product:
Core
Component:
IPC
Version:
51 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla54
Tracking Flags:
Tracking Status
firefox54 --- fixed

People

(Reporter: mishra.dhiraj95, Assigned: mrbkap)

Details

(Whiteboard: [e10s-multi:-] )

Attachments

(3 files)

tab.html
229 bytes, text/html
Details
Bug 1335989 - Split aData into smaller chunks to avoid going over the IPC message size limit.
59 bytes, text/x-review-board-request
billm
: review+
Details
Bug 1335989 - Avoid a second copy when sending substrings through IPC.
59 bytes, text/x-review-board-request
billm
: review+
Details
Attached file tab.html — 
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20170125094131

Steps to reproduce:

Steps to Reproduce:
1. Open tab.html 
2. Tab  crashes

Crash ID :
https://crash-stats.mozilla.com/report/index/aa4c689c-039e-4769-8346-e293e2170202


Actual results:

MOZ_CRASH Reason: MOZ_CRASH(IPC message size is too large)
Code :
<body onload="javascript:DoS();"></body>
  
<script>
  
function DoS() {
  
var buffer = '\x41';
for (i =0;i<666;i++) {
buffer+=buffer+'\x41';
document.write('<html><marquee><h1>'+buffer+buffer);
}
  
}
  
</script>
Again, this is just an OOM-style crash, not a security problem, like bug 1335684.
Group: firefox-core-security
I think like Gijs said that this is very similar with bug 1335684. Dhiraj based on the fact that you are the reporter from the 2 bugs do you think that they are duplicates?
Both crashes and makes the browser DOS, the ways to replicate issues are different, I believe this is an IPC crash and the bug 1335684 bug is an OOM crash.
DO you still reproduce this crash? I saw in the crash signature that there are a total of 7 crashes in 7 days and all of them happened in 2/2/2017. This is a very small number of crashes and I don't think that somebody will invest time to fix this based on the fact that the number of crashes is very small and this is not a frequent incident.
Yea I a able to reproduce the crash still.
Component: Untriaged → General
Blake, is there anything useful we can do here? If not, please close WONTFIX.

(tbh, I'm fairly sure that if we up the IPC message limit with this testcase, we'll just OOM crash elsewhere, so I don't really see the point.)
Component: General → IPC
Flags: needinfo?(mrbkap)
Product: Firefox → Core
I don't think we have to crash here.
Assignee: nobody → mrbkap
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(mrbkap)
With my patch, the testcase simply hangs in text rendering.
Whiteboard: [e10s-multi:?]
Comment on attachment 8837362 [details]
Bug 1335989 - Split aData into smaller chunks to avoid going over the IPC message size limit.

https://reviewboard.mozilla.org/r/112512/#review114340

::: netwerk/protocol/wyciwyg/WyciwygChannelChild.cpp:706
(Diff revision 1)
> +
> +  size_t curIndex = 0;
> +  size_t charsRemaining = aData.Length();
> +  do {
> +    size_t chunkSize = std::min(charsRemaining, kMaxMessageSize);
> +    SendWriteToCacheEntry(PromiseFlatString(Substring(aData, curIndex, chunkSize)));

As far as I know, you could change the type of the argument to nsDependentString. Then you could add this code to IPCMessageUtils.h:

template <>
struct ParamTraits<nsDependentString> : ParamTraits<nsAString>
{
  typedef nsDependentString paramType;
};

You'll need to change RecvWriteToCacheEntry to take an nsDependentString, but that should be fine.

Maybe this won't work, but it's worth a try.
Attachment #8837362 - Flags: review?(wmccloskey) → review+
Comment on attachment 8837851 [details]
Bug 1335989 - Avoid a second copy when sending substrings through IPC.

https://reviewboard.mozilla.org/r/112850/#review114358
Attachment #8837851 - Flags: review?(wmccloskey) → review+
Pushed by mrbkap@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5bb8a1f861b5
Split aData into smaller chunks to avoid going over the IPC message size limit. r=billm
https://hg.mozilla.org/integration/autoland/rev/c5bc41310fa2
Avoid a second copy when sending substrings through IPC. r=billm
https://hg.mozilla.org/mozilla-central/rev/5bb8a1f861b5
https://hg.mozilla.org/mozilla-central/rev/c5bc41310fa2
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Thanks blake for fixing it, does it will be eligible for bounty or CVE ?
(In reply to Dhiraj Mishra from comment #15)
> Thanks blake for fixing it, does it will be eligible for bounty or CVE ?

I marked this as not a security bug. In principle we could treat the hang/crash as one, but that would be a sec-low so likely not severe enough to warrant a bounty. Ni Al to confirm.
Flags: needinfo?(abillings)
For this particular case, Gijs is likely right, but note that the formal bug bounty process involves sending an email to security@mozilla.org for bugs you want a bounty on, and then somebody on the team that deals with the bounty program will set the sec-bounty? flag, and it will be reviewed at a weekly meeting. There are more details here:
https://www.mozilla.org/en-US/security/client-bug-bounty/
Andrew is correct. People should email us for bounty consideration. If this is a sec-low (which it should be since it is an open issue as described), then it isn't going to receive a bounty. Bugs need to be at least sec-moderate for a bounty to be awarded.
Flags: needinfo?(abillings)
Oh ok Al, so for my rest of the issues which is open should I send a summary and bug ID to that mail for bounty.
Please advice thanks.
Yes, please. Communications like this should be done by email as well. Using bugzilla as a messaging system instead of email is something to be avoided.
Hi all, If i have helped to some how to contribute in the firefox please vouch me on : 

https://mozillians.org/en-US/u/DhirajMishra/

Request Andrew, Gijs, Blake please take some time and vouch to me, this will help me a lot to be a Mozillian and will love to contribute more!

Thank you
Whiteboard: [e10s-multi:?] → [e10s-multi:-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: