83751 2012-04-12 00:32:48 -0700 [Chromium] The size of glyphStorage should be passed to substituteWithVerticalGlyphs() 2012-04-12 01:49:25 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED http://crbug.com/122585 P2 Normal --- 1 bashi bashi inferno tkent tony webkit.review.bot oldest_to_newest 600950 0 bashi 2012-04-12 00:32:48 -0700 GlyphPage::fill() in GlyphPageTreeNodeSkia.cpp calls substituteWithVerticalGlyphs() with wrong buffer length. The last argument should be the length of |glyphStorage|. 600952 1 136838 bashi 2012-04-12 00:38:04 -0700 Created attachment 136838 Patch 600954 2 bashi 2012-04-12 00:41:33 -0700 Kent-san, Tony, Could you take a look? 600958 3 136838 tkent 2012-04-12 00:49:14 -0700 Comment on attachment 136838 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136838&action=review > Source/WebCore/ChangeLog:3 > + [Chromium] Fix OOB in substituteWithVerticalGlyphs() "OOB" sounds like a security bug. > Source/WebCore/ChangeLog:11 > + No new tests. No behavior change. Really? This patch looks a fix of a problem. 600968 4 136838 bashi 2012-04-12 01:01:16 -0700 Comment on attachment 136838 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136838&action=review >> Source/WebCore/ChangeLog:3 >> + [Chromium] Fix OOB in substituteWithVerticalGlyphs() > > "OOB" sounds like a security bug. I agree. I'll change the title >> Source/WebCore/ChangeLog:11 >> + No new tests. No behavior change. > > Really? This patch looks a fix of a problem. You are right. However, I can't figure out how to test the fix. We can see the crash when we use ASAN-enabled chromium, but without ASAN, we can't see any difference at layout test level. Do you think changing the comment to describe why this patch lacks tests is enough? 600972 5 tkent 2012-04-12 01:06:50 -0700 (In reply to comment #4) > Do you think changing the comment to describe why this patch lacks tests is enough? I think so. 600976 6 136844 bashi 2012-04-12 01:10:09 -0700 Created attachment 136844 Patch 600978 7 bashi 2012-04-12 01:10:45 -0700 (In reply to comment #5) > (In reply to comment #4) > > Do you think changing the comment to describe why this patch lacks tests is enough? > > I think so. Thanks. I've updated the patch. 600980 8 136844 tkent 2012-04-12 01:11:45 -0700 Comment on attachment 136844 Patch ok 601011 9 136844 webkit.review.bot 2012-04-12 01:49:21 -0700 Comment on attachment 136844 Patch Clearing flags on attachment: 136844 Committed r113951: <http://trac.webkit.org/changeset/113951> 601012 10 webkit.review.bot 2012-04-12 01:49:25 -0700 All reviewed patches have been landed. Closing bug. 136838 2012-04-12 00:38:04 -0700 2012-04-12 01:10:04 -0700 Patch bug-83751-20120412163803.patch text/plain 1591 bashi U3VidmVyc2lvbiBSZXZpc2lvbjogMTEzOTI2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYmNkOGYyZGI1YzBhYzhl ZjM1MmZmZGUxMTA5NmE0ZGRmNDExZTlmZC4uNDE2ODYwYWUwY2M0MDNkNGY1ZjQ5M2FiMmFhODg4 YWQwOGExYTQ4ZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEyLTA0LTEyICBLZW5p Y2hpIElzaGliYXNoaSAgPGJhc2hpQGNocm9taXVtLm9yZz4KKworICAgICAgICBbQ2hyb21pdW1d IEZpeCBPT0IgaW4gc3Vic3RpdHV0ZVdpdGhWZXJ0aWNhbEdseXBocygpCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04Mzc1MQorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBsYXN0IGFyZ3VtZW50IG9mIHN1 YnN0aXR1dGVXaXRoVmVydGljYWxHbHlwaHMoKSBzaG91bGQgYmUKKyAgICAgICAgdGhlIHNpemUg b2YgfGdseXBoU3RvcmFnZXwuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLiBObyBiZWhhdmlvciBj aGFuZ2UuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9za2lhL0dseXBoUGFnZVRyZWVO b2RlU2tpYS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpHbHlwaFBhZ2U6OmZpbGwpOgorCiAyMDEy LTA0LTExICBBbnR0aSBLb2l2aXN0byAgPGFudHRpQGFwcGxlLmNvbT4KIAogICAgICAgICBUcnkg dG8gZml4IGJ1aWxkIHdpdGggWFNMVCBkaXNhYmxlZC4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJD b3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEvR2x5cGhQYWdlVHJlZU5vZGVTa2lhLmNwcCBiL1Nv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEvR2x5cGhQYWdlVHJlZU5vZGVTa2lh LmNwcAppbmRleCA5YTRmZWY2ZDBlOTgzODkyMDYwZTc3MjIzNjIzZWI4YTU3ZTJmMjY2Li45NWEy MTQ0MDAxNjVkMTQ5YzliYTIxNjRhMDdlNzk0ODUzOTRhZmU2IDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9za2lhL0dseXBoUGFnZVRyZWVOb2RlU2tpYS5jcHAK KysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9HbHlwaFBhZ2VUcmVl Tm9kZVNraWEuY3BwCkBAIC0xMDQsNyArMTA0LDcgQEAgYm9vbCBHbHlwaFBhZ2U6OmZpbGwodW5z aWduZWQgb2Zmc2V0LCB1bnNpZ25lZCBsZW5ndGgsIFVDaGFyKiBidWZmZXIsIHVuc2lnbmVkIGIK ICAgICAgICAgICAgIH0KICAgICAgICAgfQogICAgICAgICBpZiAobG9va1ZhcmlhbnRzKQotICAg ICAgICAgICAgc3Vic3RpdHV0ZVdpdGhWZXJ0aWNhbEdseXBocyhmb250RGF0YSwgZ2x5cGhzLCBi dWZmZXJMZW5ndGgpOworICAgICAgICAgICAgc3Vic3RpdHV0ZVdpdGhWZXJ0aWNhbEdseXBocyhm b250RGF0YSwgZ2x5cGhzLCBsZW5ndGgpOwogICAgIH0KIAogICAgIHVuc2lnbmVkIGFsbEdseXBo cyA9IDA7IC8vIHRyYWNrIGlmIGFueSBvZiB0aGUgZ2x5cGhJRHMgYXJlIG5vbi16ZXJvCg== 136844 2012-04-12 01:10:09 -0700 2012-04-12 01:49:21 -0700 Patch bug-83751-20120412171008.patch text/plain 1691 bashi U3VidmVyc2lvbiBSZXZpc2lvbjogMTEzOTI2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYmNkOGYyZGI1YzBhYzhl ZjM1MmZmZGUxMTA5NmE0ZGRmNDExZTlmZC4uZGRjODk2M2Q3ZWE3YjMyNTQ0NDk2NzAxZDZhNmY5 YzdjYWI1ZmVkNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEyLTA0LTEyICBLZW5p Y2hpIElzaGliYXNoaSAgPGJhc2hpQGNocm9taXVtLm9yZz4KKworICAgICAgICBbQ2hyb21pdW1d IFRoZSBzaXplIG9mIGdseXBoU3RvcmFnZSBzaG91bGQgYmUgcGFzc2VkIHRvIHN1YnN0aXR1dGVX aXRoVmVydGljYWxHbHlwaHMoKQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9ODM3NTEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBUaGUgbGFzdCBhcmd1bWVudCBvZiBzdWJzdGl0dXRlV2l0aFZlcnRpY2FsR2x5 cGhzKCkgc2hvdWxkIGJlCisgICAgICAgIHRoZSBzaXplIG9mIHxnbHlwaFN0b3JhZ2V8LgorCisg ICAgICAgIE5vIG5ldyB0ZXN0cyBiZWNhdXNlIHdlIGNhbid0IHRlc3QgdGhlIGZpeCBieSBsYXlv dXQgdGVzdHMuCisgICAgICAgIEkgY29uZmlybWVkIHRoZSBmaXggd2l0aCBBU0FOLgorCisgICAg ICAgICogcGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9HbHlwaFBhZ2VUcmVlTm9kZVNraWEuY3BwOgor ICAgICAgICAoV2ViQ29yZTo6R2x5cGhQYWdlOjpmaWxsKToKKwogMjAxMi0wNC0xMSAgQW50dGkg S29pdmlzdG8gIDxhbnR0aUBhcHBsZS5jb20+CiAKICAgICAgICAgVHJ5IHRvIGZpeCBidWlsZCB3 aXRoIFhTTFQgZGlzYWJsZWQuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9n cmFwaGljcy9za2lhL0dseXBoUGFnZVRyZWVOb2RlU2tpYS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9ncmFwaGljcy9za2lhL0dseXBoUGFnZVRyZWVOb2RlU2tpYS5jcHAKaW5kZXggOWE0 ZmVmNmQwZTk4Mzg5MjA2MGU3NzIyMzYyM2ViOGE1N2UyZjI2Ni4uOTVhMjE0NDAwMTY1ZDE0OWM5 YmEyMTY0YTA3ZTc5NDg1Mzk0YWZlNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vZ3JhcGhpY3Mvc2tpYS9HbHlwaFBhZ2VUcmVlTm9kZVNraWEuY3BwCisrKyBiL1NvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEvR2x5cGhQYWdlVHJlZU5vZGVTa2lhLmNwcApA QCAtMTA0LDcgKzEwNCw3IEBAIGJvb2wgR2x5cGhQYWdlOjpmaWxsKHVuc2lnbmVkIG9mZnNldCwg dW5zaWduZWQgbGVuZ3RoLCBVQ2hhciogYnVmZmVyLCB1bnNpZ25lZCBiCiAgICAgICAgICAgICB9 CiAgICAgICAgIH0KICAgICAgICAgaWYgKGxvb2tWYXJpYW50cykKLSAgICAgICAgICAgIHN1YnN0 aXR1dGVXaXRoVmVydGljYWxHbHlwaHMoZm9udERhdGEsIGdseXBocywgYnVmZmVyTGVuZ3RoKTsK KyAgICAgICAgICAgIHN1YnN0aXR1dGVXaXRoVmVydGljYWxHbHlwaHMoZm9udERhdGEsIGdseXBo cywgbGVuZ3RoKTsKICAgICB9CiAKICAgICB1bnNpZ25lZCBhbGxHbHlwaHMgPSAwOyAvLyB0cmFj ayBpZiBhbnkgb2YgdGhlIGdseXBoSURzIGFyZSBub24temVybwo=