RESOLVED FIXED 217249
Add maximum depth check to RedBlackTree 
https://bugs.webkit.org/show_bug.cgi?id=217249
Summary Add maximum depth check to RedBlackTree
Tadeu Zagallo
Reported 2020-10-02 14:02:52 PDT
...
Attachments
Patch (5.39 KB, patch)
2020-10-02 14:09 PDT, Tadeu Zagallo 		no flags
DetailsFormatted DiffDiff
Patch (5.75 KB, patch)
2020-10-02 15:05 PDT, Tadeu Zagallo 		no flags
DetailsFormatted DiffDiff
Patch (7.26 KB, patch)
2020-10-05 11:15 PDT, Tadeu Zagallo 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (7.26 KB, patch)
2020-10-05 12:08 PDT, Tadeu Zagallo 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (7.55 KB, patch)
2020-10-05 17:05 PDT, Tadeu Zagallo 		no flags
DetailsFormatted DiffDiff
Patch (5.79 KB, patch)
2020-10-06 17:52 PDT, Tadeu Zagallo 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Tadeu Zagallo
Comment 1 2020-10-02 14:09:20 PDT
Created attachment 410368 [details] Patch
Tadeu Zagallo
Comment 2 2020-10-02 14:10:16 PDT
<rdar://problem/69432957>
Tadeu Zagallo
Comment 3 2020-10-02 15:05:39 PDT
Created attachment 410377 [details] Patch
Mark Lam
Comment 4 2020-10-02 15:39:24 PDT
Comment on attachment 410377 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410377&action=review > Source/WTF/wtf/RedBlackTree.h:353 > + RELEASE_ASSERT(++depth <= s_maximumTreeDepth); I think this is wrong. This function iterates over the set of all nodes, not the depth of the tree. So, this check is incorrect.
Tadeu Zagallo
Comment 5 2020-10-05 11:15:27 PDT
Created attachment 410532 [details] Patch
Tadeu Zagallo
Comment 6 2020-10-05 12:08:56 PDT
Created attachment 410537 [details] Patch
Tadeu Zagallo
Comment 7 2020-10-05 17:05:44 PDT
Created attachment 410592 [details] Patch
Saam Barati
Comment 8 2020-10-06 14:15:18 PDT
Comment on attachment 410592 [details] Patch LGTM, but let's fix iterate with your idea of making it simpler
Tadeu Zagallo
Comment 9 2020-10-06 17:52:20 PDT
Created attachment 410720 [details] Patch
Darin Adler
Comment 10 2020-10-06 18:18:27 PDT
Comment on attachment 410720 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410720&action=review > Source/WTF/ChangeLog:9 > + We limit all tree traversals to 128 levels deep. That's a very conservative upper bound that Is this a security hardening measure? What motivated the change?
Saam Barati
Comment 11 2020-10-06 18:53:30 PDT
Comment on attachment 410720 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410720&action=review r=me > Source/WTF/wtf/RedBlackTree.h:353 > + RELEASE_ASSERT(++size < std::numeric_limits<unsigned>::max()); Just use Checked?
EWS
Comment 12 2020-10-07 11:05:14 PDT
Committed r268135: <https://trac.webkit.org/changeset/268135> All reviewed patches have been landed. Closing bug and clearing flags on attachment 410720 [details].
Note You need to log in before you can comment on or make changes to this bug.