Original bug here: https://bugs.webkit.org/show_bug.cgi?id=40304 Opening the page provided in the original bug: ( http://j.benzakoun.free.fr/JSNE/JavaScriptNoteEditor_b2/index.php?loadid=13&key=d16ad39b783f65e55410ab5d8ca9b4ab ) Seems to start a buffer overflow in: Web Inspector -> recources -> <file> -> contents Can confirm that this is happening on all Macs with 10.5 and up where Safari 5 or Nightly installed. Needs a check on other platforms to see if this is Mac only or not. Credit to the original reporter. Passing this on as a security bug as I don't know the risk of the cause of this bug being available in public.
This appears to be a denial-of-service (DoS) attack, or at least a performance issue. I see a lot of RAM being allocated when tapping on the "Content" tab of "index.php" in the Web Inspector, but nothing else. I think this should be duped back to Bug 40304 unless there is more information available. (Can you attach a crash log, Gustaaf?)
It is just an infinite loop in WebInspector.TextEditorModel.prototype._replaceTabsIfNeeded. I'll fix it.
(In reply to comment #2) > It is just an infinite loop in WebInspector.TextEditorModel.prototype._replaceTabsIfNeeded. I'll fix it. Does this really need to be in Security if it's an infinite loop?
(In reply to comment #3) > Does this really need to be in Security if it's an infinite loop? Removing from Security group.
Created attachment 98905 [details] Fix performance / memory allocation problem
Comment on attachment 98905 [details] Fix performance / memory allocation problem View in context: https://bugs.webkit.org/attachment.cgi?id=98905&action=review Looks good. Please fix the log entry at least. > Source/WebCore/ChangeLog:5 > + Infinite loop in WebInspector.TextEditorModel.prototype._replaceTabsIfNeeded You should change the log entry in case it was not in fact an infinite loop. Also, please start bug title with the "Web Inspector:". > Source/WebCore/inspector/front-end/TextEditorModel.js:152 > + var sum = 0; I'd rename "sum" to the "offset". > Source/WebCore/inspector/front-end/TextEditorModel.js:164 > + if (line.length > caretIndex) { no need for { } > LayoutTests/inspector/editor/text-editor-model-replace-tabs.html:15 > + InspectorTest.addResult("Output length:" + lines[0].length + ", output matches sample:" + (lines[0] === sampleOutput)); We would typically dump "text" and "sampleOutput" instead. It is way more informative.
Created attachment 99255 [details] Fix performance / memory allocation problem
Comment on attachment 99255 [details] Fix performance / memory allocation problem View in context: https://bugs.webkit.org/attachment.cgi?id=99255&action=review > Source/WebCore/ChangeLog:5 > + Performance / memory allocation issue in WebInspector.TextEditorModel.prototype._replaceTabsIfNeeded Web Inspector: prefix missing > LayoutTests/ChangeLog:5 > + Performance / memory allocation issue in WebInspector.TextEditorModel.prototype._replaceTabsIfNeeded ditto > LayoutTests/inspector/editor/text-editor-model-replace-tabs-expected.txt:16 > +[] It'd be nicer to print results side by side with the source lines. You should also render tab as \t in the source to avoid possible confusions (I have tab rendered as 8 spaces in my settings). > LayoutTests/inspector/editor/text-editor-model-replace-tabs.html:35 > + result = "FAIL: time limit (100ms) exceeded (execution time: " + duration + "ms)" This won't work on the bots. Things may take seconds there. Please do not test performance in the layout tests - they become flaky. As we agree, measuring correctness is sufficient.
Created attachment 99278 [details] Fix performance / memory allocation problem
Comment on attachment 99278 [details] Fix performance / memory allocation problem View in context: https://bugs.webkit.org/attachment.cgi?id=99278&action=review > Source/WebCore/ChangeLog:5 > + Performance / memory allocation issue in WebInspector.TextEditorModel.prototype._replaceTabsIfNeeded Please add WebInspector: prefix. > LayoutTests/ChangeLog:5 > + Performance / memory allocation issue in WebInspector.TextEditorModel.prototype._replaceTabsIfNeeded Ditto > LayoutTests/inspector/editor/text-editor-model-replace-tabs.html:12 > + var input = testCases[i]; weird indent, otherwise looks good. > LayoutTests/inspector/editor/text-editor-model-replace-tabs.html:22 > + var bufT = []; nit: no abbreviations in webkit (bufferT & bufferS)
Created attachment 99303 [details] WebInspector: Fix performance / memory allocation problem
Comment on attachment 99303 [details] WebInspector: Fix performance / memory allocation problem Clearing flags on attachment: 99303 Committed r90209: <http://trac.webkit.org/changeset/90209>
All reviewed patches have been landed. Closing bug.