Re: [PATCH] posix: _really_ make sure .hg/cache/checklink points at a real file
On 11/24/2016 12:15 AM, Mads Kiilerich wrote: On 11/23/2016 11:24 PM, Pierre-Yves David wrote: On 11/23/2016 11:09 PM, Mads Kiilerich wrote: # HG changeset patch # User Mads Kiilerich # Date 1479938505 -3600 # Wed Nov 23 23:01:45 2016 +0100 # Node ID 2841e0a6f97ba09dff5ffe7f42ac8c6e1b23338f # Parent 8836f13e3c5b8eae765372708b659c55a044cbb4 posix: _really_ make sure .hg/cache/checklink points at a real file 8836f13e3c5b failed to do what it said; it did leave a dangling symlink. As promised, that broke setup.py sdist. It also broke stuff on Solaris where "cp -r" by default follows symlinks. Instead, make it point at ../00changelog.i, which is the file that is most likely to exist. This adds some extra layering violation ... but not much, in an innocent way, and it works ... Could we just create a empty file right next to this symlink and point to that? That would seems more independant/robust. Right. If I'm understanding your reply correctly, we should expect a V2. I'm dropping this from patchwork -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] posix: _really_ make sure .hg/cache/checklink points at a real file
On 11/24/2016 12:15 AM, Mads Kiilerich wrote: On 11/23/2016 11:24 PM, Pierre-Yves David wrote: On 11/23/2016 11:09 PM, Mads Kiilerich wrote: # HG changeset patch # User Mads Kiilerich # Date 1479938505 -3600 # Wed Nov 23 23:01:45 2016 +0100 # Node ID 2841e0a6f97ba09dff5ffe7f42ac8c6e1b23338f # Parent 8836f13e3c5b8eae765372708b659c55a044cbb4 posix: _really_ make sure .hg/cache/checklink points at a real file 8836f13e3c5b failed to do what it said; it did leave a dangling symlink. As promised, that broke setup.py sdist. It also broke stuff on Solaris where "cp -r" by default follows symlinks. Instead, make it point at ../00changelog.i, which is the file that is most likely to exist. This adds some extra layering violation ... but not much, in an innocent way, and it works ... Could we just create a empty file right next to this symlink and point to that? That would seems more independant/robust. Right. Playing more around with this raise the question: Generally, when running hg commands that create/write files, how do we make sure that nobody fooled us and placed a symlink that can trick us to overwrite arbitrary files? Should we and do we always take precautions for that? Don't we have a whole vfs auditing layer especially to protect us of this class of issue? I think we should should always take precautions. I'm not too sure how this relate to the current specific issue. Did I miss something or are you expanding the discussion (which would be fine) Cheers, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] posix: _really_ make sure .hg/cache/checklink points at a real file
On 11/23/2016 11:24 PM, Pierre-Yves David wrote: On 11/23/2016 11:09 PM, Mads Kiilerich wrote: # HG changeset patch # User Mads Kiilerich # Date 1479938505 -3600 # Wed Nov 23 23:01:45 2016 +0100 # Node ID 2841e0a6f97ba09dff5ffe7f42ac8c6e1b23338f # Parent 8836f13e3c5b8eae765372708b659c55a044cbb4 posix: _really_ make sure .hg/cache/checklink points at a real file 8836f13e3c5b failed to do what it said; it did leave a dangling symlink. As promised, that broke setup.py sdist. It also broke stuff on Solaris where "cp -r" by default follows symlinks. Instead, make it point at ../00changelog.i, which is the file that is most likely to exist. This adds some extra layering violation ... but not much, in an innocent way, and it works ... Could we just create a empty file right next to this symlink and point to that? That would seems more independant/robust. Right. Playing more around with this raise the question: Generally, when running hg commands that create/write files, how do we make sure that nobody fooled us and placed a symlink that can trick us to overwrite arbitrary files? Should we and do we always take precautions for that? /Mads ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] posix: _really_ make sure .hg/cache/checklink points at a real file
Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich
> # Date 1479938505 -3600
> # Wed Nov 23 23:01:45 2016 +0100
> # Node ID 2841e0a6f97ba09dff5ffe7f42ac8c6e1b23338f
> # Parent 8836f13e3c5b8eae765372708b659c55a044cbb4
> posix: _really_ make sure .hg/cache/checklink points at a real file
>
> 8836f13e3c5b failed to do what it said; it did leave a dangling symlink. As
> promised, that broke setup.py sdist. It also broke stuff on Solaris where "cp
> -r" by default follows symlinks.
>
> Instead, make it point at ../00changelog.i, which is the file that is most
> likely to exist. This adds some extra layering violation ... but not much, in
> an innocent way, and it works ...
>
> diff --git a/mercurial/posix.py b/mercurial/posix.py
> --- a/mercurial/posix.py
> +++ b/mercurial/posix.py
> @@ -234,7 +234,8 @@ def checklink(path):
> fd = tempfile.NamedTemporaryFile(dir=checkdir,
> prefix='hg-checklink-')
> try:
> -os.symlink(os.path.basename(fd.name), name)
> +# point at some arbitrary file
> +os.symlink('../00changelog.i', name)
> if cachedir is None:
> os.unlink(name)
> else:
If you're not using fd anymore, there's no need to create it (or close it).
Danek
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] posix: _really_ make sure .hg/cache/checklink points at a real file
On 11/23/2016 11:09 PM, Mads Kiilerich wrote: # HG changeset patch # User Mads Kiilerich # Date 1479938505 -3600 # Wed Nov 23 23:01:45 2016 +0100 # Node ID 2841e0a6f97ba09dff5ffe7f42ac8c6e1b23338f # Parent 8836f13e3c5b8eae765372708b659c55a044cbb4 posix: _really_ make sure .hg/cache/checklink points at a real file 8836f13e3c5b failed to do what it said; it did leave a dangling symlink. As promised, that broke setup.py sdist. It also broke stuff on Solaris where "cp -r" by default follows symlinks. Instead, make it point at ../00changelog.i, which is the file that is most likely to exist. This adds some extra layering violation ... but not much, in an innocent way, and it works ... Could we just create a empty file right next to this symlink and point to that? That would seems more independant/robust. -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] posix: _really_ make sure .hg/cache/checklink points at a real file
# HG changeset patch
# User Mads Kiilerich
# Date 1479938505 -3600
# Wed Nov 23 23:01:45 2016 +0100
# Node ID 2841e0a6f97ba09dff5ffe7f42ac8c6e1b23338f
# Parent 8836f13e3c5b8eae765372708b659c55a044cbb4
posix: _really_ make sure .hg/cache/checklink points at a real file
8836f13e3c5b failed to do what it said; it did leave a dangling symlink. As
promised, that broke setup.py sdist. It also broke stuff on Solaris where "cp
-r" by default follows symlinks.
Instead, make it point at ../00changelog.i, which is the file that is most
likely to exist. This adds some extra layering violation ... but not much, in
an innocent way, and it works ...
diff --git a/mercurial/posix.py b/mercurial/posix.py
--- a/mercurial/posix.py
+++ b/mercurial/posix.py
@@ -234,7 +234,8 @@ def checklink(path):
fd = tempfile.NamedTemporaryFile(dir=checkdir,
prefix='hg-checklink-')
try:
-os.symlink(os.path.basename(fd.name), name)
+# point at some arbitrary file
+os.symlink('../00changelog.i', name)
if cachedir is None:
os.unlink(name)
else:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
