Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(59)

Issue 432543005: Replace NT prefix in sandbox rules match string to handle correct wildcard escaping (Closed)

Created:
6 years, 4 months ago by forshaw
Modified:
6 years, 3 months ago
CC:
chromium-reviews, wfh+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Replace NT prefix in sandbox rules match string to handle correct wildcard escaping This patch adds a function to modify file system sandbox rules to replace the \??\ NT prefix with the correct escaped form \/?/?\ for the wildcard matching rules in the broker. This is done generally as it's a common mistake in the sandbox code and so provides some defence in depth. BUG=334882 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290131

Patch Set 1 #

Patch Set 2 : Modified BUILD.gn patch as it was failing for some reason to apply. #

Patch Set 3 : Removed change to BUILD.gn due to CRLF issues in repository #

Patch Set 4 : Re-added BUILD.gn after CRLF fix #

Patch Set 5 : Reverted new unit test and added to original integration tests #

Patch Set 6 : Added missing include statement #

Total comments: 1

Patch Set 7 : Removed bug number from comment #

Total comments: 1

Patch Set 8 : Added period to end of comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -2 lines) Patch
M sandbox/win/src/filesystem_policy.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
forshaw
Looking for review on this CL. Thanks.
6 years, 4 months ago (2014-08-13 15:49:02 UTC) #1
jschuh
Looks good, but you should just tack the tests on to the end of file_policy_test.cc.
6 years, 4 months ago (2014-08-15 03:50:14 UTC) #2
forshaw
Moved the tests as requested.
6 years, 4 months ago (2014-08-16 00:20:26 UTC) #3
jschuh
The CQ bit was checked by jschuh@chromium.org
6 years, 4 months ago (2014-08-16 00:36:49 UTC) #4
jschuh
lgtm
6 years, 4 months ago (2014-08-16 00:36:49 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/forshaw@chromium.org/432543005/100001
6 years, 4 months ago (2014-08-16 00:38:16 UTC) #6
rvargas (doing something else)
https://codereview.chromium.org/432543005/diff/100001/sandbox/win/src/filesystem_policy.cc File sandbox/win/src/filesystem_policy.cc (right): https://codereview.chromium.org/432543005/diff/100001/sandbox/win/src/filesystem_policy.cc#newcode398 sandbox/win/src/filesystem_policy.cc:398: // Fixes bug: 334882 nit: we don't generally add ...
6 years, 4 months ago (2014-08-16 02:23:17 UTC) #7
commit-bot: I haz the power
Committed patchset #6 (100001) as 290131
6 years, 4 months ago (2014-08-16 08:17:18 UTC) #8
forshaw
Removed bug number from comment.
6 years, 3 months ago (2014-08-25 22:50:41 UTC) #9
rvargas (doing something else)
lgtm https://codereview.chromium.org/432543005/diff/120001/sandbox/win/src/filesystem_policy.cc File sandbox/win/src/filesystem_policy.cc (right): https://codereview.chromium.org/432543005/diff/120001/sandbox/win/src/filesystem_policy.cc#newcode397 sandbox/win/src/filesystem_policy.cc:397: // Start of name matches NT prefix, replace ...
6 years, 3 months ago (2014-08-25 23:10:12 UTC) #10
forshaw
6 years, 3 months ago (2014-08-25 23:14:13 UTC) #11
Message was sent while issue was closed.
Added period to end of comment.

Powered by Google App Engine
This is Rietveld 408576698