Follow-up Comment #13, bug #66419 (group groff): Hi Deri,
[comment #12 comment #12:]
> I am confused now! The ticket is about the restriction of including a path in
> the download file,
It's actually about several things, in my view, which I think is the only
thing confusing you.
> which you say was not intentional but was caused by some sneakiness in grops,
> but the above example shows that the path is still being rejected in the
> download file.
Yes. I have not closed this ticket or marked it as "Fixed". More work
remains.
> There is a difference between files which are groff resources, and files
> which are system resources, postscript font files in particular are a system
> resource and, as such, I don't see why including them in a document should be
> considered unsafe and require a -U flag, if that is what is being
> considered.
That was an idea of Rob's from comment #2 that I've entertained.
> The use of the type 3 postscript font as an example is a bit unfortunate,
> since Rob has explained that he organises his fonts by project, so each
> project has its own groff font directory, which can be specified by the -F
> flag, and the download file in each project directory specifies the actual
> postscript type 1 fonts required for that project, which are probably held
> centrally and the projects which require the particular font have appropriate
> entries in their download file. Please forgive me Rob if I have this all
> wrong, but it does seem an eminently sensible way of including a system
> resource into groff.
That's how I understand him as well, and I agree that it sounds thoroughly
reasonable when one is managing a library of 250,000 PostScript fonts(!)
(comment #2 again).
> I think the proper way to "fix" this is to add a bool to font::open_file,
> which, when set, allows paths in the given filename, so that when grops wants
> to open the postscript font after reading the download file, it can do the
> open call with the bool set, and all other calls to open_file can have the
> bool unset.
I think I'd prefer instead to have a new entry point in _libgroff_ called
`font::open_resource()` to make it clearer what's going on at the call site,
and omit the slash check from that. Both it and `font::open_file()` could be
backed by a static function using an additional `bool` argument to enable the
slash-rejecting logic. (I'm thinking about setting `errno`, too, since there
are 2 other failure modes arising from invariant violations: the pointers `nm`
or `device` could be null.)
> This would restore previous behaviour vis. paths in the download file, but
> retain your desire to restrict path traversal when opening groff's own
> resources.
Yes. We'll get there.
I wanted to go ahead and push some mitigators now because (a) Rob pointed out
that about 2/3rds of his debugging time was blown on dealing with the absence
of diagnostics being thrown at the call sites where they would have made
sense, and (b) something you didn't quote:
grops:<standard input>: fatal error: refusing to traverse directories to
open PostScript prologue file 'grops: ../src/libs/libgroff/errarg.cpp:112:
void errprint(const char*, const errarg&, const errarg&, const errarg&):
Assertion `!arg1.empty()' failed.
groff: error: grops: Aborted (core dumped)
...which I couldn't tolerate to let live.
Also it had been 4-5 days since my last push so I was due for one anyway.
Regards,
Branden
_______________________________________________________
Reply to this item at:
<https://savannah.gnu.org/bugs/?66419>
_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/
signature.asc
Description: PGP signature
