cgit and symlinks

[John Keeping]

On Thu, Mar 09, 2017 at 08:58:43AM +0100, MonkZ wrote:
> Am 09.03.2017 um 01:15 schrieb John Keeping:
> > On Wed, Mar 08, 2017 at 02:28:11PM +0100, MonkZ wrote:
> >>
> >>
> >> Am 08.03.2017 um 13:30 schrieb John Keeping:
> >>> On Wed, Mar 08, 2017 at 12:38:38PM +0100, MonkZ wrote:
> >>>> Am 07.03.2017 um 00:35 schrieb John Keeping:
> >>>>> We can't reliably follow the link because there is no guarantee that the
> >>>>> target lies within the repository and I don't know what we would output
> >>>>> for the case where we can't display the target.
> >>>>
> >>>> INADH (I'm not a dev here)
> >>>>
> >>>> I would recommend to continue ignoring it or returning the blob, because
> >>>> following symlinks (internally) might result -  if not done carefully -
> >>>> in directory traversal security issues. Maybe resolving a symlink to a
> >>>> HTTP301 could work.
> >>>>
> >>>> For the UI there might be a html-link (in a notification box "This is a
> >>>> symlink that points to ...") to the symlink-destination below or above
> >>>> the blob, to get a user via click to a file/directory.
> >>>
> >>> We're talking about the "plain" UI here (for example [0]), so we don't
> >>> have anywhere to put additional content and it has to be something
> >>> basic.
> >> Of course. It would be handled like a content-rewrite to return a http301.
> >>
> >> Pseudocode:
> >> handle_symlinks = True # new config item
> >> if this_file_is_a_symlink and symlink_is_relative and handle_symlinks:
> >> 	if plain_ui:
> >> 		# rewrite blob to http301
> >> 		# by attaching the path to the end of current basedir
> >> 		# cgit is already able to handle ../ in a path
> >> 	if !plain_ui:
> >> 		# show blob
> >> 		# show notification that this is a symlink
> >> 		# show a link to a url
> >> 		# 	like the one that would be used in plain_ui
> >>
> >>>
> >>> I'm not actually too worried about directory traversal if we were to try
> >>> following links because we're looking things up in a Git tree at a
> >>> particular commit and not on the filesystem.  A bigger concern would be
> >>> whether the internals of Git do anything bad (like invalid memory
> >>> access) if we give the tree traversal machinery a path that goes up out
> >>> of the repository; I doubt it but I have not checked.
> >> If we use url-rewrites (and let the http-client care about getting the
> >> correct file or directory), this would be a non-issue.
> > 
> > It could also mean that cross-repository symlinks work if the server
> > layout matches that that is expected for checkouts of the repositories.
> > 
> > But it's not exactly helpful if a repository contains an absolute
> > symlink and I don't think we want to start figuring out whether a
> > redirect makes sense - what do we do if we decide it doesn't?
> > 
> 
> Absolute symlinks must be ignored. There is no deterministic way to
> resolve them - every clone can be at a different location, and there
> isn't really a deterministic mapping from url to filesystem. Absolute
> symlinks would only work if resolved internally - with additional
> security risks.
> 
> Relative inter-repository links may allowed/handled/redirected if
> explicitly configured, otherwise it might be confusing if the server
> layout doesn't match. On the other hand a notification "This is a
> symlink outside this repository" might suffice (but i don't have a plan
> for plain-ui).

We can consider improvements to the tree UI separately, but I really
don't think we should be getting into anything clever with symlinks in
the plain UI because it ends up with complicated rules like the above.

It's difficult to explain and will end up surprising users, so my
preference is for my original patch that just displays the content of
the blob when a symlink is found.  This is consistent with both
"git show" and "git cat-file".