New issue
Advanced search Search tips

Issue 771359 link

Starred by 1 user
Status: Fixed
Owner:
jrumm...@chromium.org
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 3
Type: Bug



Sign in to add a comment

CDM allows invalid characters in names of saved files

Project Member Reported by jrumm...@chromium.org, Oct 3 2017 
Currently the CDM API for cdm::FileIO notes:
  // - |file_name| must not contain forward slash ('/') or backslash ('\'), and
  //   must not start with an underscore ('_').

However, if you try to rename a file on Windows to include a slash, it displays:
  A file name can't contain any of the following characters:
    \ / : * ? " < > |
As well, on Mac : is a reserved character and should not be used in file names.

I don't think any CDMs currently use any of these illegal characters in the names of files that get saved, but we should implement stronger verification.

Proposal:
Change the comment (and implementation) so that the only characters allowed in |file_name| are regular ASCII characters (A-Za-z), digits (0-9), and a small number of special characters (say -_.).

The restriction that they can't start with _ will remain, as that is used by pepper when creating a temporary file.
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 18 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/cdm/+/d9e4d645bc720a531c5fd5290960787281dce8b7

commit d9e4d645bc720a531c5fd5290960787281dce8b7
Author: John Rummell <jrummell@chromium.org>
Date: Fri Oct 06 20:05:04 2017

Limit file_name passed to FileIO::Open() to a restricted set of characters

Previously |file_name| was limited to not containing / or \. However, to
ensure the file is allowable across all file systems, limit |file_name| to
be only letters (A-Za-z), digits(0-9), or "._-". It also must be between 1
and 256 characters long.

The previous restriction that it does not start with _ remains.

BUG= 771359 

Change-Id: I0f695daeb98e76b019bff9657424b99a229a0b23

[modify] https://crrev.com/d9e4d645bc720a531c5fd5290960787281dce8b7/content_decryption_module.h

Comment 2 by jrumm...@chromium.org, Nov 3 2017 

This has been implemented for mojo files [1].

[1] https://cs.chromium.org/chromium/src/media/mojo/services/mojo_cdm_file_io.cc?l=32
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 10 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e76bb16c1a2e491f7a17bf5381cc7156a220a75e

commit e76bb16c1a2e491f7a17bf5381cc7156a220a75e
Author: John Rummell <jrummell@chromium.org>
Date: Fri Nov 10 00:08:24 2017

media: Stronger file name checking on file names used by the CDM

To ensure the file name is allowable on all file systems, limit it to be
only letters (A-Za-z), digits(0-9), or "._-". It also must be between 1
and 256 characters long.

Current CDMs do not currently use names that violate these new restrictions,
so there is no problem with existing saved files.

BUG= 771359 
TEST=encrypted media browser_tests pass

Change-Id: Id5926c7391f18336718bbd4b145c2bf88968c704
Reviewed-on: https://chromium-review.googlesource.com/759736
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Commit-Queue: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515352}
[modify] https://crrev.com/e76bb16c1a2e491f7a17bf5381cc7156a220a75e/media/cdm/ppapi/cdm_file_io_impl.cc
[modify] https://crrev.com/e76bb16c1a2e491f7a17bf5381cc7156a220a75e/media/cdm/ppapi/cdm_file_io_test.cc

Comment 4 by jrumm...@chromium.org, Nov 10 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment