pdf/post crash with long glyph names

[vuori]

FontAwesome version 5 contains glyph names which are longer than GNLEN (32). This causes a strcpy buffer overflow in font_glyphput.

Fix for the immediate problem is to increase GNLEN, but perhaps checking the length and skipping the offending glyph with a warning would be nice here?

[aligrudi]

vuori notifications@github.com wrote:

FontAwesome version 5 contains glyph names which are longer than GNLEN (32). This causes a strcpy buffer overflow in font_glyphput.

Fix for the immediate problem is to increase GNLEN, but perhaps checking the length and skipping the offending glyph with a warning would be nice here?

Now Neatpost truncates glyph names at GNLEN, like Neatroff. Also, Neatmkfn warns if glyph names are truncated.

AFAIK, glyph names in OTF are limited to 64 bytes. Maybe we should increase GNLEN?

Ali

[vuori]

Upping to 64 sounds good, that's what I set it to off the cuff. The offending FontAwesome glyph was called american-sign-language-interpreting (35 bytes).

[aligrudi]

vuori notifications@github.com wrote:

Upping to 64 sounds good, that's what I set it to off the cuff. The offending FontAwesome glyph was called american-sign-language-interpreting (35 bytes).

Increasing GNLEN would probably increase Neatroff's memory usage noticeably. If such glyphs appear rarely, maybe we can safely let GNLEN remain 32. We can also remove ILNLEN. Please test the following patch.

Thanks, Ali

diff --git a/dev.c b/dev.c index 10a6783..469fb79 100644 --- a/dev.c +++ b/dev.c @@ -77,7 +77,7 @@ int dev_mnt(int pos, char id, char name) int dev_open(char dir, char dev) { char path[PATHLEN];

• char tok[ILNLEN];
• char tok[128]; int i; FILE desc; snprintf(dev_dir, sizeof(dev_dir), "%s", dir); @@ -86,7 +86,7 @@ int dev_open(char dir, char *dev) desc = fopen(path, "r"); if (!desc) return 1;
• while (fscanf(desc, "%s", tok) == 1) {
• while (fscanf(desc, "%128s", tok) == 1) { if (tok[0] == '#') { skipline(desc); continue; @@ -99,7 +99,7 @@ int dev_open(char dir, char dev) continue; } if (!strcmp("sizes", tok)) {
• while (fscanf(desc, "%s", tok) == 1)

• while (fscanf(desc, "%128s", tok) == 1) if (!strcmp("0", tok)) break; continue; diff --git a/dir.c b/dir.c index af65222..d4cbd13 100644 --- a/dir.c +++ b/dir.c @@ -77,7 +77,7 @@ static void setcolor(int m)

  void dir_fix(struct sbuf sbuf, char src) {

• char cmd[ILNLEN];
• char cmd[1024]; char prev_s = src; char r, c; int f = -1, s = -1, m = -1; diff --git a/font.c b/font.c index 14a3ec2..04e78a9 100644 --- a/font.c +++ b/font.c @@ -327,16 +327,16 @@ int font_layout(struct font fn, struct glyph *gsrc, int nsrc, int sz, static int font_readchar(struct font fn, FILE fin, int n, int gid) { struct glyph g;
• char tok[ILNLEN];
• char name[ILNLEN];
• char id[ILNLEN];
• char tok[128];
• char name[GNLEN];
• char id[GNLEN]; int type;
• if (fscanf(fin, "%s %s", name, tok) != 2)
• if (fscanf(fin, GNFMT " %128s", name, tok) != 2) return 1; if (!strcmp("---", name)) sprintf(name, "c%04d", *n); if (strcmp("\"", tok)) {
• if (fscanf(fin, "%d %s", &type, id) != 2)
• if (fscanf(fin, "%d " GNFMT, &type, id) != 2) return 1; gid = font_glyphput(fn, id, name, type); g = &fn->gl[gid]; @@ -472,13 +472,13 @@ static int font_readgsub(struct font fn, FILE fin) struct grule *rule; int feat, scrp, lang; int i, n;
• if (fscanf(fin, "%s %d", tok, &n) != 2)
• if (fscanf(fin, "%128s %d", tok, &n) != 2) return 1; font_readfeat(fn, tok, &feat, &scrp, &lang); rule = font_gsub(fn, n, feat, scrp, lang); rule->sec = fn->secs; for (i = 0; i < n; i++) {
• if (fscanf(fin, "%s", tok) != 1)
• if (fscanf(fin, "%128s", tok) != 1) return 1; if (tok[0] == '-') rule->pats[i].flg = GF_PAT; @@ -499,13 +499,13 @@ static int font_readgpos(struct font fn, FILE fin) struct grule *rule; int feat, scrp, lang; int i, n;
• if (fscanf(fin, "%s %d", tok, &n) != 2)
• if (fscanf(fin, "%128s %d", tok, &n) != 2) return 1; font_readfeat(fn, tok, &feat, &scrp, &lang); rule = font_gpos(fn, n, feat, scrp, lang); rule->sec = fn->secs; for (i = 0; i < n; i++) {
• if (fscanf(fin, "%s", tok) != 1)

• if (fscanf(fin, "%128s", tok) != 1) return 1; col = strchr(tok, ':'); if (col) @@ -523,12 +523,12 @@ static int font_readgpos(struct font fn, FILE fin)

  static int font_readggrp(struct font fn, FILE fin) {

• char tok[ILNLEN];
• char tok[GNLEN]; int id, n, i, g; if (fscanf(fin, "%d %d", &id, &n) != 2) return 1; for (i = 0; i < n; i++) {
• if (fscanf(fin, "%s", tok) != 1)

• if (fscanf(fin, GNFMT, tok) != 1) return 1; g = font_idx(fn, font_glyph(fn, tok)); if (g >= 0) @@ -539,10 +539,10 @@ static int font_readggrp(struct font fn, FILE fin)

  static int font_readkern(struct font fn, FILE fin) {

• char c1[ILNLEN], c2[ILNLEN];
• char c1[GNLEN], c2[GNLEN]; struct grule *rule; int val;
• if (fscanf(fin, "%s %s %d", c1, c2, &val) != 3)
• if (fscanf(fin, GNFMT " " GNFMT " %d", c1, c2, &val) != 3) return 1; rule = font_gpos(fn, 2, font_findfeat(fn, "kern"), -1, -1); rule->pats[0].g = font_idx(fn, font_glyph(fn, c1)); @@ -605,7 +605,7 @@ struct font font_open(char path) struct font fn; int ch_g = -1; / last glyph in the charset / int ch_n = 0; / number of glyphs in the charset */
• char tok[ILNLEN];
• char tok[128]; FILE fin; char ligs[512][GNLEN]; int ligs_n = 0; @@ -623,7 +623,7 @@ struct font font_open(char *path) fn->ch_dict = dict_make(-1, 1, 0); fn->ch_map = dict_make(-1, 1, 0); fn->ggrp = iset_make();
• while (fscanf(fin, "%s", tok) == 1) {

• while (fscanf(fin, "%128s", tok) == 1) { if (!strcmp("char", tok)) { font_readchar(fn, fin, &ch_n, &ch_g); } else if (!strcmp("kern", tok)) { diff --git a/hyph.c b/hyph.c index 64e602e..da0b973 100644 --- a/hyph.c +++ b/hyph.c @@ -296,7 +296,7 @@ void hyphenate(char hyph, char word, int flg)

  void tr_hpfa(char **args) {

• char tok[ILNLEN], c1[ILNLEN], c2[ILNLEN];
• char tok[128], c1[GNLEN], c2[GNLEN]; FILE filp; hyinit = 1; / load english hyphenation patterns with no arguments */ @@ -306,21 +306,21 @@ void tr_hpfa(char *args) } / reading patterns */ if (args[1] && (filp = fopen(args[1], "r"))) {
• while (fscanf(filp, "%s", tok) == 1)
• while (fscanf(filp, "%128s", tok) == 1) if (strlen(tok) < WORDLEN) hy_add(tok); fclose(filp); } / reading exceptions / if (args[2] && (filp = fopen(args[2], "r"))) {
• while (fscanf(filp, "%s", tok) == 1)
• while (fscanf(filp, "%128s", tok) == 1) if (strlen(tok) < WORDLEN) hw_add(tok); fclose(filp); } / reading hcode mappings / if (args[3] && (filp = fopen(args[3], "r"))) {
• while (fscanf(filp, "%s", tok) == 1) {

• while (fscanf(filp, "%128s", tok) == 1) { char s = tok; if (utf8read(&s, c1) && utf8read(&s, c2) && !s) hcode_add(c2, c1); / inverting / diff --git a/roff.h b/roff.h index 9c4472f..c81b496 100644 --- a/roff.h +++ b/roff.h @@ -28,10 +28,10 @@

  define NFONTS 32 / number of fonts /

  define FNLEN 32 / font name length /

  define GNLEN 32 / glyph name length /

  +#define GNFMT "%32s" / glyph name scanf format /

  define NMLEN 128 / macro/register/environment name length /

  define RNLEN NMLEN / register/macro name /

  define NREGS 8192 / number of mapped names /

  -#define ILNLEN 1000 / line limit of input files /

  define NARGS 32 / number of macro arguments /

  define NPREV 16 / environment stack depth /

  define NTRAPS 1024 / number of traps per page /

  diff --git a/sbuf.c b/sbuf.c index f03ddd6..426c83f 100644 --- a/sbuf.c +++ b/sbuf.c @@ -38,7 +38,7 @@ void sbuf_append(struct sbuf sbuf, char s)

  void sbuf_printf(struct sbuf sbuf, char s, ...) {

• char buf[ILNLEN];
• char buf[1024]; va_list ap; va_start(ap, s); vsnprintf(buf, sizeof(buf), s, ap);

[vuori]

After applying the patch to neatroff and reverting the increase to GNLEN in neatpost, I'm still seeing the crash with a long glyph name:

#6  0x000055555555c3bd in strcpy (__src=0x7fffffffd520 "american-sign-language-interpreting", __dest=<optimized out>)
    at /usr/include/x86_64-linux-gnu/bits/string_fortified.h:90
#7  font_glyphput (type=3, name=0x7fffffffd130 "", id=0x7fffffffd520 "american-sign-language-interpreting", fn=0x5555558d9400)
    at font.c:44

[aligrudi]

vuori notifications@github.com wrote:

After applying the patch to neatroff and reverting the increase to GNLEN in neatpost, I'm still seeing the crash with a long glyph name:

#6  0x000055555555c3bd in strcpy (  src=0x7fffffffd520 "american-sign-language-interpreting",   dest=<optimized out>)
    at /usr/include/x86 64-linux-gnu/bits/string fortified.h:90
#7  font glyphput (type=3, name=0x7fffffffd130 "", id=0x7fffffffd520 "american-sign-language-interpreting", fn=0x5555558d9400)
    at font.c:44

I have updated both Neatroff and Neatpost (no need for the patch anymore). Please test it once more. Do you get the same result?

Thanks, Ali

[vuori]

Working with the latest changes, thanks.