Re: rewrite try_dlopen (1)

[Ralf Wildenhues]

Hi Gary,

* Gary V. Vaughan wrote on Mon, Oct 04, 2004 at 01:34:05PM CEST:
> Ralf Wildenhues wrote:
> >
> >  But for the syntax, I'm thinking along these lines:
> > 
> > - it must be Bourne-shell source-able.
> > - A line is either
> >     empty
> >     a comment (starts with #)
> >     or a (complete!) variable assignment.
> > - The right-hand side of a variable assignment should either be
> >     single-quoted
> >     or not quoted at all (and consist of one word only).
> 
> There are some double quoted fields in the .la files I just glanced at
> (relink_command in this case), though there is no reason that particular case
> couldn't use single quotes.  I think it is a good idea to specify that to
> reduce surprises from shell expansion.

I was not so sure about that (the fact that single quotes suffice).
But alas, since there is usage now, we have to support it anyway.

> > - The libltdl parsing *must* yield the same value as shell sourcing
> >   in case that
> >     no variable expansions are used ($foo)
> >     no shell wildcards are used (libfoo*)
> >     no config.status expansions are used (@foo@)
> 
> Are you saying that these constructs should not be used?  I'm not sure I agree
> with that, and again I've found @inst_prefix_dir@ in a libltdl.la file, so we
> use that construct at least.
> 
> Maybe you're just saying that the shell sourcing can only be identical to the
> parser read value when these constructs are not used.  I do agree with that.

The latter.  For now I want to keep these expansions out of the ltdl
code.  Fine with me to use them for `libtool'.  But then we must specify
which variables are (un)interesting for ltdl.

Usage within `libtool' can (and in fact does right now) undergo
globbing, variable expansion and such.  Basically all three of the above
I mentioned.

> >   All these cases I'd like to leave undefined (as a possible way of
> >   extending the defined syntax later on) for libltdl at least.
> > - libltdl should not crash even on bogus .la files.  This requirement
> >   stems from the goal of third-parties eventually being able to write
> >   such files.  Besides, I have a tendency of thinking of all input files
> >   as unverified (thus, treat them as potentially malicious).
> 
> Most definitely.  We should take a lot more care to security audit the code of
> a new parser against accidentally running malicious code from a .la file.  I'm
> not sure there is much we can do when sourcing the .la file from the libtool
> script... maybe we should grep out assignments, and only source those lines.
> Hmmm, then again, I don't think that buys us much.  If someone wants to
> subvert a .la file, they will need to change the content of the variables that
> are run by root (like the relink_command).

I agree.  There is no real security implication here, anyone getting in
our library path has already won.  What I'd like to prevent is segfaults
(assertions are segfaults, too; production code ignores assert()) that
stem from buggy files.  They should preferably return a meaningful
error.  In fact a patch adding an error is pending, one reason for my
question about binary incompatibility (which I'd still like to have
answered more conclusively).

> > Any objections against these?  I think they are backwards-compatible in
> > the sense that newer libltdl should be able to read older .la files this
> > way.
> 
> Apart from my comments above (which are probably just a misunderstanding on my
> part), no objections from me.

OK.

> > BTW, the rewrite process has already revealed more than one of crashing
> > the parsing code...  and I hate things such as sensitivity to trailing
> > white space (e.g. `installed=yes ').
> 
> I'm not at all surprised.  I still don't quite understand how the inner
> recursive calls of tryall_dlopen interact :-(

Getting closer.

Regards,
Ralf