[2/2] mov: Double-check that alias path does is not an absolute path
diff mbox

Message ID 1428411966-24973-2-git-send-email-vittorio.giovara@gmail.com
State Committed
Commit 9286de045968ad456d4e752651eec22de5e89060
Headers show

Commit Message

Vittorio Giovara April 7, 2015, 1:06 p.m. 
nlvl_to and nlvl_from can be set to 1 if both alias and target files
are in the same directory, so actually check the first character of the
string.
---
 libavformat/mov.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Diego Biurrun April 7, 2015, 1:13 p.m. | #1 
s/does is/is/

Diego
Diego Biurrun April 10, 2015, 8:25 p.m. | #2 
On Tue, Apr 07, 2015 at 03:06:05PM +0200, Vittorio Giovara wrote:
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -2300,7 +2300,7 @@ static int mov_open_dref(AVIOContext **pb, char *src, MOVDref *ref,
>      /* try relative path, we do not try the absolute because it can leak information about our
>         system to an attacker */
> -    if (ref->nlvl_to > 0 && ref->nlvl_from > 0) {
> +    if (ref->nlvl_to > 0 && ref->nlvl_from > 0 && ref->path[0] != '/') {

Won't this work only on Unix?

Diego
Vittorio Giovara April 12, 2015, 3:48 p.m. | #3 
On Fri, Apr 10, 2015 at 10:25 PM, Diego Biurrun <diego@biurrun.de> wrote:
> On Tue, Apr 07, 2015 at 03:06:05PM +0200, Vittorio Giovara wrote:
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -2300,7 +2300,7 @@ static int mov_open_dref(AVIOContext **pb, char *src, MOVDref *ref,
>>      /* try relative path, we do not try the absolute because it can leak information about our
>>         system to an attacker */
>> -    if (ref->nlvl_to > 0 && ref->nlvl_from > 0) {
>> +    if (ref->nlvl_to > 0 && ref->nlvl_from > 0 && ref->path[0] != '/') {
>
> Won't this work only on Unix?
>
> Diego

Afaik only UNIX style paths are stored in this atom.
Vittorio Giovara April 12, 2015, 4:17 p.m. | #4 
On Sun, Apr 12, 2015 at 5:48 PM, Vittorio Giovara
<vittorio.giovara@gmail.com> wrote:
> On Fri, Apr 10, 2015 at 10:25 PM, Diego Biurrun <diego@biurrun.de> wrote:
>> On Tue, Apr 07, 2015 at 03:06:05PM +0200, Vittorio Giovara wrote:
>>> --- a/libavformat/mov.c
>>> +++ b/libavformat/mov.c
>>> @@ -2300,7 +2300,7 @@ static int mov_open_dref(AVIOContext **pb, char *src, MOVDref *ref,
>>>      /* try relative path, we do not try the absolute because it can leak information about our
>>>         system to an attacker */
>>> -    if (ref->nlvl_to > 0 && ref->nlvl_from > 0) {
>>> +    if (ref->nlvl_to > 0 && ref->nlvl_from > 0 && ref->path[0] != '/') {
>>
>> Won't this work only on Unix?
>>
>> Diego
>
> Afaik only UNIX style paths are stored in this atom.

Let me rephrase, in this atom, when type == 2 paths are stored MacOS
style, so with : instead of /, and we take care of the : to /
conversion, when type == 18 paths are stored in UNIX style, so we just
copy the path there.
In one way or another an absolute path will always start with / to my
knowledge. This section in the spec is very poorly documented.

Patch
diff mbox 

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 5f577f3..f35f06d 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2300,7 +2300,7 @@  static int mov_open_dref(AVIOContext **pb, char *src, MOVDref *ref,
 {
     /* try relative path, we do not try the absolute because it can leak information about our
        system to an attacker */
-    if (ref->nlvl_to > 0 && ref->nlvl_from > 0) {
+    if (ref->nlvl_to > 0 && ref->nlvl_from > 0 && ref->path[0] != '/') {
         char filename[1024];
         char *src_path;
         int i, l;