www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 3420] New: Impossible to specify -J path for subdirectories

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420

           Summary: Impossible to specify -J path for subdirectories
           Product: D
           Version: 1.042
          Platform: All
        OS/Version: Windows
            Status: NEW
          Severity: regression
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: thecybershadow gmail.com



---
const data = import("dir/data.txt");

Specifying -J. for DMD 1.041 is sufficient to allow this to compile.

I couldn't find an option for DMD 1.042 and newer which would allow this to
compile.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 19 2009
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |bugzilla digitalmars.com
         Resolution|                            |WONTFIX



02:36:43 PST ---
Paths are not allowed in the string supplied to the import statement. This is
for security reasons. To get the example to compile, use:

    import ("data.txt");

and use the switch:

    -Jdir

This behavior is as intended.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 30 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420


Vladimir <thecybershadow gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|WONTFIX                     |



---
That's fine and dandy, except it doesn't work either:

C:\Temp\D\Bug3420> dir
 Volume in drive C is WS2008X64
 Volume Serial Number is 4CC8-8E34

 Directory of C:\Temp\D\Bug3420

2009.11.30  12:38    <DIR>          .
2009.11.30  12:38    <DIR>          ..
2009.11.30  12:38    <DIR>          dir
2009.11.30  12:38                38 test.d
               1 File(s)             38 bytes
               3 Dir(s)  11,197,788,160 bytes free

C:\Temp\D\Bug3420> dir dir
 Volume in drive C is WS2008X64
 Volume Serial Number is 4CC8-8E34

 Directory of C:\Temp\D\Bug3420\dir

2009.11.30  12:38    <DIR>          .
2009.11.30  12:38    <DIR>          ..
2009.11.30  12:38                 4 data.txt
               1 File(s)              4 bytes
               2 Dir(s)  11,197,788,160 bytes free

C:\Temp\D\Bug3420> cat test.d
const data = import("dir/data.txt");

C:\Temp\D\Bug3420> dmd -Jdir test.d
test.d(1): Error: use -Jpath switch to provide path for filename dir/data.txt

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 30 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420


Leandro Lucarella <llucax gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |llucax gmail.com



PST ---
I think you have to do this:
const data = import("data.txt");
                     ^
                 no "dir/"
$ dmd -Jdir test.d

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 30 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420




---
Ah, I see. This should be clarified in the documentation... Also, doesn't
anyone think that this could be too constricting? What if you have a directory
tree of data to import? Not to mention not being able to import two files with
the same filename but from different directories...

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 30 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420




PST ---


 Ah, I see. This should be clarified in the documentation... Also, doesn't
 anyone think that this could be too constricting?


I do.

Maybe this can be changed to be a feature request. I really can't see how
allowing subdirectories can be a security risk, you only have to check that the
canonical name of the imported file is still in a subdirectory of an -J'ed
directory.

In POSIX you can use realpath(3) to get the canonical name of a file, then just
check if the imported canonical name starts with any -J directory canonical
name.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 30 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420




PST ---
Created an attachment (id=520)
Proof of concept patch

Here it is a proof of concept patch to allow directories in string imports
*safely*. The check is done as I said in comment 5: -J paths are converted to
canonical names, then the string import path is appended and the resulting path
is again converted to a canonical name. Then, the canonical name is checked to
be really in the canonical path. This prevents any type of highjacking (even
with symlinks).

Here is a simple example:

import("x/../../y") in combination with -J.. (assuming /tmp/x is the current
directory) is checked like this:
1) .. is converted to realpath(..) which yields /tmp
2) the canonical path is combined with the file name: /tmp/x/../../y
3) the new filename is converted to a canonical filename: /y
4) the canonical path and the canonical name are checked: /y doesn't start with
/tmp, so the import is rejected.

Unfortunately, I'm not a windows developer, and the path is only implemented
for POSIX (and only tested in Linux, but if other *nixes don't work it should
be fairly simple to fix). Compiling in Windows yields an error for now. If
there is no way to implement this on Windows, it's fairly easy to allow this
behavior in POSIX and fallback to the old behavior in Windows. Let me know if
you want a patch for that.

I'll attach a few test cases.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 30 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420


Leandro Lucarella <llucax gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch
           Severity|regression                  |enhancement


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 30 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420




PST ---
Created an attachment (id=521)
Test cases

Here are some test cases, they are packed in a tarball because it includes a
directory structure.

Tests are in the directory d2, and they should be compiled in that directory.
The tests have a small comment indicating what -J should be used to compile
(other -J options should fail).

You can test them all with this simple bash script (run it in the d2
directory):
for f in test{1..7}.d
do
    for j in . ..
    do
        echo -n "$f: "; head -n1 $f
        dmd -J$j $f && ./`basename $f .d`
    done
done

You have to go through the results and check them visually, the script can be
improve to make the verification automatically.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 30 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420




PST ---
BTW, the patch I consider the patch only a proof of concept because it lacks
Windows support (and testing on other unixes). Besides that, I think the patch
is not bad =)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 30 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420




---
It's really frustrating when such unannounced changes in compiler behavior
break my code. Today I needed to fix up an old program I wrote, which makes
extensive usage of importing files from a subdirectory tree. I couldn't use an
old compiler version due to bugs... needless to say, I wasted time editing
import paths and creating special-case build scripts. An undocumented breaking
of functionality hardly fits a "stable" implementation...

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Dec 03 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420


Leandro Lucarella <llucax gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|1.042                       |1.006
           Severity|enhancement                 |regression



PST ---
So, this is *really* a regression then, I'm sorry I changed the severity. But
are you sure it works in DMD 1.041? I tracked this down in the code and it was
introduced at DMD 1.006, when the -J option got implemented, so I can't see how
it could work in DMD 1.041:
http://www.dsource.org/projects/dmd/changeset/121
http://www.dsource.org/projects/dmd/changeset/121#file4 (expression.c)
http://www.dsource.org/projects/dmd/changeset/121#file11 (mars.c)
http://www.digitalmars.com/d/1.0/changelog.html#new1_006

Before that, you could use string imports in a very promiscuous way. The specs
are very brief about string imports and doesn't say anything about
restrictions:

  The AssignExpression must evaluate at compile time to a constant string. The
  text contents of the string are interpreted as a file name. The file is read,
  and the exact contents of the file become a string literal. 

So, the -J option seems to be an implementation-defined behavior, as such, I
guess it should be documented. For D1, I don't know if the specs should be
changed, but I guess the old behavior should work (the attached patch can be
used to allow secure string imports without prohibiting sub-directories
altogether).

I don't know if the compiler option shouldn't be documented too, it seems
pretty useless to make this an implementation defined behavior, you'll have to
stay always with the most restrictive implementation limitations if you want to
write code that can be used with any compiler, or avoid using this feature at
all.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Dec 03 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420




PST ---
Yes, I was very careful in finding the exact version this problem was
introduced. As stated in the issue description, the problem manifests in DMD
1.042 (and newer), but not 1.041. At first I thought it was an accidental
regression, caused by "Fix bug where / wasn't recognized as a path separator on
Windows", but as Walter has stated, this was an intentional change. 

And yes, I have already agreed that this needs to be better documented.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Dec 03 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420




PST ---
I just tried DMD 1.041 and I can't make

const data = import("dir/data.txt");

work using -J. (or any other -J option for that matter). Can you try again with
DMD 1.041? I can't see any change in DMD post 1.006 that can affect this
behavior. The limitation was introduced in that version of the compiler.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Dec 04 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420




PST ---
C:\Downloads\dmd.1.041\dmd\windows\bin> dmd | head -n 1
Digital Mars D Compiler v1.041

C:\Downloads\dmd.1.041\dmd\windows\bin> type test.d
const data = import("dir/data.txt");

C:\Downloads\dmd.1.041\dmd\windows\bin> dir dir
 Volume in drive C is WS2008X64
 Volume Serial Number is 4CC8-8E34

 Directory of C:\Downloads\dmd.1.041\dmd\windows\bin\dir

2009.12.04  18:32    <DIR>          .
2009.12.04  18:32    <DIR>          ..
2009.12.04  18:32                 0 data.txt
               1 File(s)              0 bytes
               2 Dir(s)  84,268,494,848 bytes free

C:\Downloads\dmd.1.041\dmd\windows\bin> dmd -c -J. test
(no output, compiles successfully)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Dec 04 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420




PST ---
Doesn't work on Linux, so this is a Windows-only problem (well, it was a
Windows-only fix really :). Here is the fix:
http://www.dsource.org/projects/dmd/changeset/156#file64

This is the entry in the DMD 1.042 changelog that fixed this problem:
* Fix bug where / wasn't recognized as a path separator on Windows.

Try with import("dir\\data.txt"), it should fail to compile in DMD 1.006+.

So, summarizing:

* DMD 1.006 introduces a regression, which is supposed to be a feature.
* Supposing this is a real feature, you were relying on a bug in your programs,
if you code them with DMD 1.006+ in the first place.
* The specs are not clear about this, reading the specs there is no reason to
think that import("a/b") shouldn't work (i.e., at least there should be a
compiler switch to make it work or the specs should be fixed to reflect the
reality).
* There is a better solution to relax the current restrictions without being
completely promiscuous about string imports (see attached patch).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Dec 04 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420




PST ---
So, my programs relied on a bug in DMD, which was recently rightfully fixed.



Well, that sucks.
Hoping your patch is accepted.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Dec 05 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420




22:59:36 PST ---
Changeset 396, for Posix only.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 21 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420




22:20:35 PST ---
Posix-only fix in dmd 1.057 and 2.041

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Mar 08 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|patch                       |
                 CC|                            |clugdbug yahoo.com.au



Removing 'patch' keyword -- there's no patch for the systems for which the
issue still applies.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 07 2010
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3420


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[PATCH] Allow string import |Allow string import of
                   |of files using              |files using subdirectories
                   |subdirectories              |
           Severity|regression                  |enhancement



This link:

https://www.securecoding.cert.org/confluence/display/seccode/FIO02-C.+Canonicalize+path+names+originating+from+untrusted+sources

states that:

"Producing canonical file names for Windows operating systems is extremely
complex and beyond the scope of this standard. The best advice is to try to
avoid making decisions based on a path, directory, or file name [Howard 2002].
Alternatively, use operating-system-based mechanisms, such as access control
lists (ACLs) or other authorization techniques."

Thus, this issue might not be fixable on Windows. 
I'm downgrading this all the way from 'regression' to 'enhancement', since it
was a security bug that it ever worked at all. Perhaps the bug should just be
closed.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 20 2010