File content corruption isn't detected in for 0 byte corruption, unreadable file, or missing file |
||
Issue descriptionThere are 3 cases of file corruption/deletion in extensions I've found that is not detected by our code (primarily by URLRequestExtensionJob): 1) If a file is 0 bytes, URLRequestExtensionJob never sees OnReadComplete. 2) If a file is missing, we should be overriding URLRequestFileJob::OnOpenComplete and detect that. 3) If a file doesn't have read permissions (can happen during corruption), #2 applies. The result is that since URLRequestExtensionJob::OnReadComplete never fires, ContentVerifyJob::DoneReading() doesn't get called and content verification never kicks in. Notes about #1: a 0 byte file doesn't necessarily mean that it's due to corruption. An extension can legitimately have a 0 byte file in it.
,
Mar 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d6dbb26e31818eed0097654ecb89aaddc2c64f8d commit d6dbb26e31818eed0097654ecb89aaddc2c64f8d Author: lazyboy <lazyboy@chromium.org> Date: Thu Mar 30 00:57:30 2017 Fix content verification code for undreadable and deleted files. It seems that corruption examples in crbug.com/699420 can cause extension resource files to go missing or become unreadable. Fix a bug where we never sent any notification to ContentVerifyJob, in particular: we never called ContentVerifyJob::DoneReading(). Override URLRequestFileJob::OnOpenComplete in URLRequestExtensionJob to catch these cases. Also make ContentHashReader not skip legitimate deleted files: the files that are listed in verified_contents.json. It will still skip non-existent resources, e.g. xhrs requesting non-existent file (see crbug.com/404802 for example). BUG= 703888 Test=Install an extension (not app) in chromeos. Chromeos because you want content verification enforcement to kick in. Other way could be to use --extension-content-verification=enforce_strict flag in other platform. Either: 1) Delete background script file. Content verifcation would kick in when you try to load/run the extension. 2) Same as above^^^, instead of deleting the file, remove the file permission. In linux you can just do "chmod -r background.js". Expect content verification failure. In both cases extension should be reinstalled in the end. Review-Url: https://codereview.chromium.org/2771953003 Cr-Commit-Position: refs/heads/master@{#460607} [modify] https://crrev.com/d6dbb26e31818eed0097654ecb89aaddc2c64f8d/chrome/browser/extensions/content_verifier_browsertest.cc [modify] https://crrev.com/d6dbb26e31818eed0097654ecb89aaddc2c64f8d/chrome/browser/extensions/extension_protocols_unittest.cc [modify] https://crrev.com/d6dbb26e31818eed0097654ecb89aaddc2c64f8d/extensions/browser/BUILD.gn [modify] https://crrev.com/d6dbb26e31818eed0097654ecb89aaddc2c64f8d/extensions/browser/content_hash_reader.cc [modify] https://crrev.com/d6dbb26e31818eed0097654ecb89aaddc2c64f8d/extensions/browser/content_hash_reader.h [modify] https://crrev.com/d6dbb26e31818eed0097654ecb89aaddc2c64f8d/extensions/browser/content_verify_job.cc [modify] https://crrev.com/d6dbb26e31818eed0097654ecb89aaddc2c64f8d/extensions/browser/content_verify_job.h [add] https://crrev.com/d6dbb26e31818eed0097654ecb89aaddc2c64f8d/extensions/browser/content_verify_job_unittest.cc [modify] https://crrev.com/d6dbb26e31818eed0097654ecb89aaddc2c64f8d/extensions/browser/extension_protocols.cc [modify] https://crrev.com/d6dbb26e31818eed0097654ecb89aaddc2c64f8d/extensions/browser/scoped_ignore_content_verifier_for_test.h [add] https://crrev.com/d6dbb26e31818eed0097654ecb89aaddc2c64f8d/extensions/test/data/content_hash_fetcher/with_verified_contents/README.txt [add] https://crrev.com/d6dbb26e31818eed0097654ecb89aaddc2c64f8d/extensions/test/data/content_hash_fetcher/with_verified_contents/source_all.zip
,
Mar 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e83ab9c6c27deb3e96dee4a33a6518c206724d16 commit e83ab9c6c27deb3e96dee4a33a6518c206724d16 Author: lazyboy <lazyboy@chromium.org> Date: Thu Mar 30 03:18:26 2017 Make content verification see 0-byte files. Currently ContentVerifyJob never sees DoneReading() for (legitimate or not) 0-byte files. This means 0-byte files (legitimate or corrupted) never goes through content verification process. Override URLRequestJob::DoneReading() in URLRequestExtensionJob to detect the end of 0-byte files so that ContentVerifyJob knows about these 0-byte files. Add a test to ensure that URLRequestExtensionJob will see 0-byte files. ContentVerifyJobUnittest.DeletedAndMissingFiles already makes sure that 0-byte read will fail content verification. BUG= 703888 , 703892 Test=With content verification turned on (e.g. with --extension-content-verification=enforce_strict flag), install an extension from webstore and modify a JS content inside that extension to 0 byte file. Content verification should fail for that extension. If the extension is part of system policy, the extension will be reinsalled. Review-Url: https://codereview.chromium.org/2779083005 Cr-Commit-Position: refs/heads/master@{#460638} [modify] https://crrev.com/e83ab9c6c27deb3e96dee4a33a6518c206724d16/chrome/browser/extensions/extension_protocols_unittest.cc [modify] https://crrev.com/e83ab9c6c27deb3e96dee4a33a6518c206724d16/extensions/browser/extension_protocols.cc
,
Mar 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6665648f820bad8de48fc0699ba315696b6dd127 commit 6665648f820bad8de48fc0699ba315696b6dd127 Author: lazyboy <lazyboy@chromium.org> Date: Thu Mar 30 07:07:53 2017 Add a ContentVerifyJob test for sanity checking legitimate 0 byte file. There is no restriction that an extension can upload zero byte resource. Add a test to test that behavior to make sure content verificaton works as expected. Take the opportunity to refactor repeated code into RunContentVerifyJob function. BUG= 703888 Test=No visible change. Review-Url: https://codereview.chromium.org/2786803002 Cr-Commit-Position: refs/heads/master@{#460685} [modify] https://crrev.com/6665648f820bad8de48fc0699ba315696b6dd127/extensions/browser/content_verify_job_unittest.cc [add] https://crrev.com/6665648f820bad8de48fc0699ba315696b6dd127/extensions/test/data/content_hash_fetcher/zero_byte_file/README.txt [add] https://crrev.com/6665648f820bad8de48fc0699ba315696b6dd127/extensions/test/data/content_hash_fetcher/zero_byte_file/source.zip
,
Apr 4 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by lazyboy@chromium.org
, Mar 21 2017