Apply missing ReferencesParent check

Apply missing kParentDirectory check

BUG=161564
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=168692

[Tom Sepez, 2012-11-17 02:15:30]

Adding @pima for OWNER.

[Tom Sepez, 2012-11-17 02:20:44]

Actually, @darin is an owner and has context.  Please review.

[piman, 2012-11-19 17:56:05]

http://codereview.chromium.org/11414046/diff/7001/content/browser/child_proce...
File content/browser/child_process_security_policy_impl.cc (right):

http://codereview.chromium.org/11414046/diff/7001/content/browser/child_proce...
content/browser/child_process_security_policy_impl.cc:148: return false;
TBH, without more context, this seems overly restrictive. Could you give me some
context, maybe add me to the bug?

[Tom Sepez, 2012-11-19 18:02:18]

@piman - you've been added to bug 161564.

[piman, 2012-11-19 18:23:51]

Is this called only for renderers, or for, say, Pepper too? If only renderers,
then LGTM, otherwise, we should probably make sure to always "resolve" path
names before applying the policy.

[Tom Sepez, 2012-11-19 19:24:57]

On 2012/11/19 18:23:51, piman wrote:
> Is this called only for renderers, or for, say, Pepper too? If only renderers,
> then LGTM, otherwise, we should probably make sure to always "resolve" path
> names before applying the policy.

I wouldn't want to preclude the possibility, since it would seem reasonable for
Pepper to honor .. so long as we don't traverse outside of a blessed directory.

There doesn't appear to be a FilePath method at present for doing this.
Searching through code, it seems that you're on your own for doing this (e.g.
there's a spot in extensions where they do this (and their code is
wrong/vulnerable as it turns out)).

I'll put an implementation into CPSP, but we should consider fixing this in
file_path.cc itself.

[Tom Sepez, 2012-11-19 20:32:52]

Revised Patch.

[piman, 2012-11-19 20:57:57]

LGTM, and thanks for the test.
Your right, I think the canonicalization step would be useful in FilePath (hard
to get right/safe).

[Tom Sepez, 2012-11-19 22:15:01]

Per discussion, it sounds like we now want to go with the earlier patch set #3. 
Please confirm.

[piman, 2012-11-19 22:33:53]

On 2012/11/19 22:15:01, Tom Sepez wrote:
> Per discussion, it sounds like we now want to go with the earlier patch set
#3. 
> Please confirm.

After we verified all the callers and (if necessary) do canonicalization in the
calling processes (renderer? pepper?)

[Tom Sepez, 2012-11-19 22:40:48]

Ok.  I'll land #4 and we can work on backtracking to #3.  Need to get this hole
closed quickly.

[commit-bot: I haz the power, 2012-11-19 22:41:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tsepez@chromium.org/11414046/8006

[commit-bot: I haz the power, 2012-11-19 23:57:36]

Retried try job too often for step(s) interactive_ui_tests

[commit-bot: I haz the power, 2012-11-20 01:08:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tsepez@chromium.org/11414046/8006

[commit-bot: I haz the power, 2012-11-20 01:54:00]

Change committed as 168692