bugGNU roff - Bugs: bug #64577, [grops] can't embed/download fonts...

 
 

bug #64577: [grops] can't embed/download fonts from a subdirectory

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Mon 21 Aug 2023 09:58:32 AM UTC
   
 
Category:  Driver grops Severity:  4 - Important
Item Group:  Warning/Suspicious behaviour Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.24.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 01 Dec 2023 08:15:43 PM UTC, comment #15: 

Kicking up to "important" severity, per comment #12 ("we should have mentioned this (and produced these better diagnostics) when 1.23.0 was released.  Distributors may find this change desirable to backport.").

G. Branden Robinson <gbranden>
Group administrator
Wed 30 Aug 2023 06:10:52 PM UTC, comment #14: 

comment #12:

> So let us spin this into a new ticket.


Spun into bug #64612.

Dave <barx>
Group Member
Thu 24 Aug 2023 12:00:39 AM UTC, comment #13: 

comment #12:

> I'm starting to think a new environment variable, GROFF_RESOURCE_PATH, might be necessary to handle the sorts of file system objects that grops and gropdf (and only these output drivers, as far as I know), sometimes require.

The ghostscript approach suits my needs to share Type 1 fonts sensibly between multiple unrelated applications.  I'm optimistic it's likely to be able to address the specific design and security constraints.

comment #12:

> So let us spin this into a new ticket.  Phil, do you want to remain CCed on that?

Yes please.

Phil Chadwick <gpw928>
Wed 23 Aug 2023 09:43:45 AM UTC, comment #12: 


commit c08b62fd2a812e4d390ebf65d6c7d337d512d2b4
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Aug 21 06:35:49 2023 -0500

    [grops]: Fix Savannah #64577 (file diagnostics).

    * src/devices/grops/ps.cpp (ps_printer::define_encoding):
    * src/devices/grops/psrm.cpp (resource_manager::output_prolog)
      (resource_manager::supply_resource): Report more intelligible
      diagnostics when libgroff's `font::open_file()` returns a null pointer
      without setting `errno`.  The only way this can happen is if it
      rejected the file name for containing a slash, thus attempting
      directory traversal (recall Savannah #61424).  Also fix code style
      nits: explicitly `#include` errno.h C standard library header, align
      style of null pointer checks, and stop explicitly setting `errno` to
      zero before (indirectly) calling `fopen()`; we inspect `errno`'s value
      only under a documented error condition (a null stream pointer).  See
      errno(3).

    * NEWS: Add item; we should have mentioned this (and produced these
      better diagnostics) when 1.23.0 was released.  Distributors may find
      this change desirable to backport.

    Fixes <https://savannah.gnu.org/bugs/?64577>.  Thanks to Phil Chadwick
    for the report and Deri James for swiftly finding a correct workaround
    that suited the reporter.


However, as comment #11 notes, we may not be done here.

I'm starting to think a new environment variable, GROFF_RESOURCE_PATH, might be necessary to handle the sorts of file system objects that grops and gropdf (and only these output drivers, as far as I know), sometimes require.

Whether such a new environment variable should be treated as a counterpart to output drivers' `-I` option as GROFF_FONT_PATH already is for `-F` is an open question.  I think we got into trouble in the first place by overloading the semantics of `GROFF_FONT_PATH` (and `-F`) with "embeddable resources".  Really, it's better that this overloading not be done, because `font::open_file()` is particularized to the needs of a device-independent troff--the reading of device-specific description files (including those for fonts).

What PostScript and PDF do with resources is a different kettle of fish, and neither the formatter nor the output driver need to understand them, or not much.  I recognize we do have some support for extracting bounding boxes/media boxes and stuff like that.

So let us spin this into a new ticket.  Phil, do you want to remain CCed on that?


G. Branden Robinson <gbranden>
Group administrator
Wed 23 Aug 2023 03:12:32 AM UTC, comment #11: 

I decided to tidy up the Type 1 fonts and put them all the pfa, pfb, and ps files into /usr/local/share/fonts/Type1, which is where X11 (and numerous other applications) expect to find them.

[These path names are FreeBSD specific, but Linux will be analogous.]

Everything went fine until I started forefox.  It went to 100% CPU. So I tried chrome, and iridium, all with the same result.

Truss showed that they were stuck in an infinite loop repeatedly traversing the devps "directory" (/usr/local/share/fonts/Type1/devps -> .). It seems that these web browsers can't handle a self-referential sym link.
 
Luckily I provide Type 1 fonts to groff in "ps" format, and I can put them back into a separate directory, not used by web browsers.  So I can call it "devps" or I can add the "devps -> ." symlink. 
 
But, I expect that many users will want groff to be able to use pfa fonts also used by X11 applications like web browsers.  The "devps -> ." hack won't work for them.

Nobody will want to keep a separate copy of Type 1 fonts just for the sole use of groff.

I understand you have a security issue.  May I suggest you review the meaning of GROFF_FONT_PATH and its requirement to contain a directory named  "devps".  Maybe consider looking at ghostscript's GS_LIB (either a single directory or a list of directories separated by a character appropriate for the operating system) as a starting point model?

Phil Chadwick <gpw928>
Tue 22 Aug 2023 02:19:32 PM UTC, comment #10: 

Thanks for correcting my addled advice regarding `-F` and $GROFF_FONT_PATH, Deri!

G. Branden Robinson <gbranden>
Group administrator
Mon 21 Aug 2023 11:50:51 PM UTC, comment #9: 

comment #6:

> Problem solved.  I'm grateful and impressed.  Please close the issue.


Glad your issue is solved!

Your initial report did identify a subpar diagnostic, for which Branden posted the comment #2 patch.  So I'm leaving this open for now; we keep Branden around with the periodic dopamine jolts he gets from closing tickets as resolved.

Dave <barx>
Group Member
Mon 21 Aug 2023 11:25:24 PM UTC, comment #8: 

You said you were using;-

> [strand.202] $ export GROFF_FONT_PATH=/usr/local/share/fonts/Type1Fonts
> [strand.203] $ groff -F $GROFF_FONT_PATH -Tps e.nr >/dev/null


No need to alter it, just add the symlink, I should have been clearer.

Deri James <deri>
Group Member
Mon 21 Aug 2023 11:11:01 PM UTC, comment #7: 

I just realised that GROFF_FONT_PATH needs to contain a directory called devps.  What I did worked because of the circular sym link to devps, but a better setting is:

GROFF_FONT_PATH=/usr/local/share/fonts/Type1Fonts


Phil Chadwick <gpw928>
Mon 21 Aug 2023 10:53:50 PM UTC, comment #6: 

Deri's comment #4 makes the environment variable work:

[strand.144] $ grep -i groff $HOME/.profile
groff_fp=/usr/local/share/fonts/Type1Fonts/devps
[ -d "$groff_fp" ] && GROFF_FONT_PATH="$groff_fp" && export GROFF_FONT_PATH

[strand.145] $ ls -lad $GROFF_FONT_PATH
lrwxr-xr-x  1 root  bin  1 Aug 22 08:04 /usr/local/share/fonts/Type1Fonts/devps -> .

[strand.669] $ cd /usr/local/share/groff/1.23.0/font/devps
[strand.670] $ grep Matrix download
Dot-Matrix      Dot-Matrix.ps

[strand.672] $ groff -Tps e.nr >e.ps
[strand.673] $ grep Matrix e.ps
%%+ font Dot-Matrix
%%BeginResource: font Dot-Matrix
%!PS-AdobeFont-1.0: Dot-Matrix 1.000
FontDirectory/Dot-Matrix known{/Dot-Matrix findfont dup/UniqueID known{dup
/FullName(Dot Matrix)readonly def
/FamilyName(Dot Matrix)readonly def
/FontName /Dot-Matrix def
/FontMatrix[0.001 0 0 0.001 0 0]readonly def
/Dot-Matrix@0 ENC0/Dot-Matrix RE
/F0 10/Dot-Matrix@0 SF(Hello)72 12 Q(world)7.5 E 0 Cg EP


The Type 1 fonts are now downloading into the document.

I double checked that the FreeEuro \[ue] was also working (the freeeuro.pfa file download from the native "devps" directory) and it was.

Problem solved.  I'm grateful and impressed.  Please close the issue.

Thanks.

Phil Chadwick <gpw928>
Mon 21 Aug 2023 07:24:18 PM UTC, comment #5: 

From a design perspective, this does seem like the situation -U was meant for: telling groff "I'm not processing some random file from sketchy-web-site.com, but something I wrote myself, so I know it's not doing anything nefarious."

Still, if Deri's comment #4 makes the environment variable work for Phil, no code need change other than the diagnostic sharpening.

Dave <barx>
Group Member
Mon 21 Aug 2023 04:03:47 PM UTC, comment #4: 

The reason why -F and GROFF_FONT_PATH failed is because groff expects this to point to a directory containing a directory called devps which contains the fonts for postscript. So it should work if you create a symlink in the Type1Fonts directory called devps which points to itself (i.e. ".").

Deri James <deri>
Group Member
Mon 21 Aug 2023 02:01:54 PM UTC, comment #3: 

Thank you for your very prompt attention.

I have confirmed that if the postscript font to be downloaded is in the devps directory (and I modify the download file to reflect that), groff works correctly.

If I set the GROFF_FONT_PATH to the directory where the postscript font files normally reside, groff reports an error:


[strand.202] $ export GROFF_FONT_PATH=/usr/local/share/fonts/Type1Fonts
[strand.203] $ groff -F $GROFF_FONT_PATH -Tps e.nr >/dev/null
grops:<standard input>:19: error: failed to open PostScript resource 'Dot-Matrix.ps': No such file or directory
[strand.204] $ ls -lad $GROFF_FONT_PATH/*Matrix*
-rw-r--r--  1 bin  bin  333921 Aug 29  1998 /usr/local/share/fonts/Type1Fonts/Dot-Matrix.ps
[strand.207] $ grep Matrix download
Dot-Matrix      Dot-Matrix.ps

My PostScript fonts are used by multiple applications.  They are not the exclusive property of groff, and having to put them all in a directory private to groff is a deeply inconvenient solution.  All applications use a sym link to a directories containing the various Type 1 font files (afm, pfb, pfm, and ps).

Setting an environment variable to point to the downloadable fonts would be acceptable, but that method seems not to work at the moment (see above).

Phil Chadwick <gpw928>
Mon 21 Aug 2023 11:10:24 AM UTC, comment #2: 

Proposed patch for diagnostic issue.  Swept up related cases with it.


diff --git a/src/devices/grops/ps.cpp b/src/devices/grops/ps.cpp
index 33fce911c..a8cdd2f11 100644
--- a/src/devices/grops/ps.cpp
+++ b/src/devices/grops/ps.cpp
@@ -788,8 +788,13 @@ void ps_printer::define_encoding(const char *encoding, int encoding_index)
     vec[i] = 0;
   char *path;
   FILE *fp = font::open_file(encoding, &path);
-  if (fp == 0)
+  if (fp == 0) {
+    // If errno not valid, assume file rejected due to '/'.
+    if (errno <= 0)
+      fatal("refusing to traverse directories to open PostScript"
+           " encoding file '%1'");
     fatal("can't open encoding file '%1'", encoding);
+  }
   int lineno = 1;
   const int BUFFER_SIZE = 512;
   char buf[BUFFER_SIZE];
diff --git a/src/devices/grops/psrm.cpp b/src/devices/grops/psrm.cpp
index 3c9a8b7b9..0c4dcc8ed 100644
--- a/src/devices/grops/psrm.cpp
+++ b/src/devices/grops/psrm.cpp
@@ -316,9 +316,14 @@ void resource_manager::output_prolog(ps_output &out)
   }
   char *prologue = getenv("GROPS_PROLOGUE");
   FILE *fp = font::open_file(prologue, &path);
-  if (!fp)
-    fatal("failed to open PostScript prologue '%1': %2", prologue,
+  if (!fp) {
+    // If errno not valid, assume file rejected due to '/'.
+    if (errno <= 0)
+      fatal("refusing to traverse directories to open PostScript"
+           " prologue file '%1'");
+    fatal("failed to open PostScript prologue file '%1': %2", prologue,
          strerror(errno));
+  }
   fputs("%%BeginResource: ", outfp);
   procset_resource->print_type_and_name(outfp);
   putc('\n', outfp);
@@ -354,8 +359,13 @@ void resource_manager::supply_resource(resource *r, int rank,
     if (r->type == RESOURCE_FONT) {
       fp = font::open_file(r->filename, &path);
       if (!fp) {
-       error("failed to open PostScript resource '%1': %2",
-             r->filename, strerror(errno));
+       // If errno not valid, assume file rejected due to '/'.
+       if (errno <= 0)
+         error("refusing to traverse directories to open PostScript"
+               " resource file '%1'");
+       else
+         error("failed to open PostScript resource file '%1': %2",
+               r->filename, strerror(errno));
        delete[] r->filename;
        r->filename = 0 /* nullptr */;
       }


G. Branden Robinson <gbranden>
Group administrator
Mon 21 Aug 2023 10:31:24 AM UTC, comment #1: 

Hi Phil,

Thank you for this excellently documented bug report.  Please follow up to the Savannah bug tracker at <https://savannah.gnu.org/bugs/?64577>.

original submission:


>     [strand.488] $ groff -Tps e.nr >e.ps
>     grops:<standard input>:(e.nr):20: error: failed to open PostScript resource
> 'Type1Fonts/Dot-Matrix.ps': No error: 0


An error message that looks like this always means a programmer goof, so at the very least we have a defective diagnostic.
 

> That error message is new to groff version 1.23.0.

[...]

> I think it's a bug.  Sincerely hoping it's not a "feature".


It is fallout from a different change that was deliberate, and documented, but which understandably you would likely not have seen coming even with a careful reading of our NEWS file and release notes.


o The `fp` request no longer accepts file or font names with slashes in
  them as arguments.  All font description files are expected to be
  accessible within the directory of the output device for which they
  were prepared.

o afmtodit no longer writes file names with directory information in
  them to the "name" directives of the font descriptions it generates.
  (The `fp` request no longer accepts such names; see "troff" above.)


We made this change because we were concerned about the security implications of directory traversal.  See bug #61424.

The missing information is that the `fp` formatter request uses a function from an internal library, `font::open_file()`, to locate and access a font description file.

Something I missed was that grops (and only grops) uses it too.


src/devices/grops/ps.cpp:  FILE *fp = font::open_file(encoding, &path);
src/devices/grops/psrm.cpp:  FILE *fp = font::open_file(prologue, &path);
src/devices/grops/psrm.cpp:      fp = font::open_file(r->filename, &path);
src/devices/grops/psrm.cpp:  FILE *fp = font::open_file("download", &path);


As a workaround and test, please move your Dot-Matrix.ps file into /usr/local/share/groff/1.23.0/font/devps (or make a symlink), retry, and let us know if that solves the problem.

If it does, and that is satisfactory for you, then I will confine the scope of this ticket to the unhelpful diagnostic message.

Please also test adding your Type1 subdirectory to groff's font search path.  There are a couple of ways of doing that.


export GROFF_FONT_PATH=/usr/local/share/groff/1.23.0/font/devps/Type1
groff -F /usr/local/share/groff/1.23.0/font/devps/Type1 -Tps e.nr >e.ps


(Or as a variant on the first, you could set the environment variable at the beginning of the command-line to confine its scope, rather than exporting it into your shell's environment.)

This/these should also work, and I definitely want to know if it/they don't.

If the foregoing is not satisfactory, and you want grops to be able to access arbitrary directories relative to its "home" in $DESTDIR/share/groff/1.23.0/font/devps, then I will need to consult with my fellow groff developers for perspectives on the wisdom and security implications of doing so.  This might mean adding a `-U` ("unsafe") flag to grops and would then still require a change to your workflow, because you'd then need to pass it.  This can be done with the `-P -U` option sequence to the groff command.

One of the reasons I think it likely that we won't go back to groff 1.22.4 behavior here is that the formatter both permits arbitrary PostScript to be injected into an input document (via the `device` and `devicem` requests and `\X` and `\Y` escape sequences).  So we probably don't want to ship a grops that will grub around in arbitrary places on the file system.  Even though it's not privileged anyway.  roff documents have a MIME type and it's no fun for people to open attachments in their mailer and get a pop-up PostScript renderer exclaiming "im in ur /etc readin ur passwd" and seeing your actual /etc/passwd file contents subsequently.

Please let me know what you discover with respect to the workarounds, and share your perspective on the safety issue.


G. Branden Robinson <gbranden>
Group administrator
Mon 21 Aug 2023 09:58:32 AM UTC, original submission:  

Filing on behalf of Phil Chadwick <phil.chadwick@claremont.farm>, who reported it to the bug-groff list.

*

I have just discovered that as of GNU grops (groff) version 1.23.0
groff no longer downloads and encodes PostScript fonts into documents.

This is the method by which I create fully self-contained PostScript
documents that can be viewed or printed anywhere (because all the fonts
are encoded within the document).

The method by which this is done is to configure access to the PostScript
fonts in the devps/download file by appending the font name and location
(relative to the devps directory), e.g.


    [strand.178] $ uname -a
    FreeBSD strand.my.dom 13.2-RELEASE-p2 FreeBSD 13.2-RELEASE-p2 GENERIC amd64

    [strand.148] $ cd /usr/local/share/groff/1.23.0/font/devps

    [strand.149] $ cat download
    # PostScript-name       Filename
    # These three come standard with groff
    Symbol-Slanted          symbolsl.pfa
    ZapfDingbats-Reverse    zapfdr.pfa
    FreeEuro                freeeuro.pfa

    Dot-Matrix      Type1Fonts/Dot-Matrix.ps


Thyere is also a font metrics file:


    [strand.150] $ head -10 Dot-Matrix
    name Dot-Matrix
    internalname Dot-Matrix
    spacewidth 750
    encoding text.enc
    charset
    ha  750,803 2       0000    asciicircum
    ti  750,803 2       0001    asciitilde
    :Y  750,803,53      2       0006    Ydieresis
    tm  750,803,303     3       0007    trademark
    aq  750,803 2       0010    quotesingle


And a Postscript font:


    [strand.158] $ file Type1Fonts/Dot-Matrix.ps
    Type1Fonts/Dot-Matrix.ps: PostScript Type 1 font text (Dot-Matrix 1.000)


In the past a "\f[Dot-Matrix]" directive would cause groff to encode the
Dot-Matrix.ps font file into the PostScript document it produced.


    [strand.487] $ cat e.nr
    \f[Dot-Matrix]Hello world

    [strand.488] $ groff -Tps e.nr >e.ps
    grops:<standard input>:(e.nr):20: error: failed to open PostScript resource
'Type1Fonts/Dot-Matrix.ps': No error: 0


That error message is new to groff version 1.23.0.

The font Dot-Matrix is not encoded into the output file:


    [strand.142] $ grep Dot-Matrix e.ps
    %%DocumentNeededResources: font Dot-Matrix
    %%IncludeResource: font Dot-Matrix
    /Dot-Matrix@0 ENC0/Dot-Matrix RE
    /F0 10/Dot-Matrix@0 SF(Hello)72 12 Q(world)7.5 E 0 Cg EP


With GNU grops (groff) version 1.22.4, I get what I want and expect:


    [sherman.134] $ grep Dot-Matrix e.ps
    %%+ font Dot-Matrix
    %%BeginResource: font Dot-Matrix
    %!PS-AdobeFont-1.0: Dot-Matrix 1.000
    FontDirectory/Dot-Matrix known{/Dot-Matrix findfont dup/UniqueID known{dup
    /FontName /Dot-Matrix def
    /Dot-Matrix@0 ENC0/Dot-Matrix RE
    /F0 10/Dot-Matrix@0 SF(Hello)72 12 Q(world)3.33 E 0 Cg EP


The groff version 1.23.0 e.ps file:

    - displays correctly on an X11 screen using gv, probably because
      ghostscript knows where to find the pfb files for the Type 1 fonts.

    - will not print correctly (the PostScript printer knows nothing about
      the Dot-Matrix font), nor can I send the document to someone else to
      view (because they won't have the Dot-Matrix font).

I think it's a bug.  Sincerely hoping it's not a "feature".

G. Branden Robinson <gbranden>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by barx (Posted a comment)
  • -email is unavailable- added by deri (Posted a comment)
  • -email is unavailable- added by gpw928 (Posted a comment)
  • -email is unavailable- added by gbranden (Submitted the item)
  • -email is unavailable- added by gbranden (original reporter)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-12-01 gbranden Severity3 - Normal 4 - Important
    2023-08-23 gbranden Item GroupIncorrect behaviour Warning/Suspicious behaviour
    2023-08-23 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.24.0
    2023-08-21 barx StatusNeed Info In Progress
    2023-08-21 gbranden StatusNone Need Info
        Assigned toNone gbranden
    2023-08-21 gbranden Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-bb6a.
    Corresponding source code