NetBSD Problem Report #54069

From www@netbsd.org  Sat Mar 23 16:36:34 2019
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 5820B7A186
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 23 Mar 2019 16:36:34 +0000 (UTC)
Message-Id: <20190323163633.78ABC7A1CB@mollari.NetBSD.org>
Date: Sat, 23 Mar 2019 16:36:33 +0000 (UTC)
From: thorpej@me.com
Reply-To: thorpej@me.com
To: gnats-bugs@NetBSD.org
Subject: mandoc(1) appears to have a use-after-free bug
X-Send-Pr-Version: www-1.0

>Number:         54069
>Category:       bin
>Synopsis:       mandoc(1) appears to have a use-after-free bug
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Mar 23 16:40:00 +0000 2019
>Closed-Date:    Mon Apr 08 18:31:11 +0000 2019
>Last-Modified:  Mon Apr 08 18:31:11 +0000 2019
>Originator:     Jason Thorpe
>Release:        NetBSD-current sources from 20190319
>Organization:
NetBSD
>Environment:
macOS 10.13.6 host, cross building -current sources noted above.
>Description:
#    format  man/groff_ms.html7
if test "" != "yes"; then  /nbsd/tools/bin/nbmandoc -Thtml -Oman=../html%S/%N.html,style=../style.css  groff_ms.7 > groff_ms.html7.tmp &&  mv -f groff_ms.html7.tmp groff_ms.html7;  else  GROFF_ENCODING=  GROFF_BIN_PATH=/nbsd/tools/lib/groff  GROFF_FONT_PATH=/nbsd/tools/share/groff/site-font:/nbsd/tools/share/groff/font  GROFF_TMAC_PATH=/nbsd/tools/share/groff/site-tmac:/nbsd/tools/share/groff/tmac /nbsd/tools/bin/nbgroff  -Tlatin1 -mdoc2html groff_ms.7   > groff_ms.html7.tmp && mv -f groff_ms.html7.tmp groff_ms.html7;  fi
nbmandoc(66926,0x7fff8ce6f380) malloc: *** error for object 0x7fc980c4aa50: incorrect checksum for freed object - object was probably modified after being freed.
*** set a breakpoint in malloc_error_break to debug
/bin/sh: line 1: 66926 Abort trap: 6           /nbsd/tools/bin/nbmandoc -Thtml -Oman=../html%S/%N.html,style=../style.css groff_ms.7 > groff_ms.html7.tmp

*** Failed target:  groff_ms.html7

>How-To-Repeat:
Best way I know how is to cross-build a release from a macOS 10.13.6 host; the macOS malloc() on my system detects the error.
>Fix:

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54069 CVS commit: src/external/bsd/mdocml/dist
Date: Thu, 28 Mar 2019 16:26:49 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Thu Mar 28 20:26:49 UTC 2019

 Modified Files:
 	src/external/bsd/mdocml/dist: out.c

 Log Message:
 PR/54069: Jason Thorpe: Prevent memory overrun. Can be easily reproduced
 with groff_ms.7 and -fsanitize=address.


 To generate a diff of this commit:
 cvs rdiff -u -r1.3 -r1.4 src/external/bsd/mdocml/dist/out.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Ingo Schwarze <schwarze@usta.de>
To: gnats-bugs@NetBSD.org
Cc: Thomas Klausner <wiz@NetBSD.org>, Christos Zoulas <christos@netbsd.org>,
	Jason Thorpe <horpej@me.com>
Subject: Re: bin/54069
Date: Fri, 29 Mar 2019 22:46:20 +0100

 Hi,

 thanks to Jason for finding and reporting the bug, to Thomas for
 making me aware of the PR, and to Christos for his commit, which
 helped me understanding the bug.

 I just fixed it upstream:

   http://mandoc.bsd.lv/cgi-bin/cvsweb/out.c#rev1.78

 Note that my patch is smaller than Christos' and at a different
 place.  It is easier to understand because that is where maxcol is
 originally calculated.  Changing it later when it has already been
 used for various purposes is somewhat confusing.   Maybe you want
 to merge the upstream patch, to ease future updates and to avoid
 ultimately ending up with a double fix.

 Thanks,
   Ingo


 Log Message:
 -----------
 Set the maximum column index in a tbl(7) to the maximum *right* edge 
 of any cell span, not to the maximum *left* edge, which may be smaller 
 if the last column of the table is only reached by horizontal spans,
 but not by any regular cell in any row of the table.
 Otherwise, the algorithm calculating column widths accessed memomy
 after the end of the colwidth[] array, while it was trying to handle 
 the rightmost column(s).

 Crash reported by Jason Thorpe <thorpej at NetBSD>
 via https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=54069
 and via Thomas Klausner (wiz@).
 Christos@ Zoulas sent a (correct, but slightly confusing) patch.
 The patch i'm committing here is easier to understand.

 Modified Files:
 --------------
     mandoc:
         out.c

 Revision Data
 -------------
 Index: out.c
 ===================================================================
 RCS file: /home/cvs/mandoc/mandoc/out.c,v
 retrieving revision 1.77
 retrieving revision 1.78
 diff -Lout.c -Lout.c -u -p -r1.77 -r1.78
 --- out.c
 +++ out.c
 @@ -149,7 +149,7 @@ tblcalc(struct rofftbl *tbl, const struc
  		gp = &first_group;
  		for (dp = sp->first; dp != NULL; dp = dp->next) {
  			icol = dp->layout->col;
 -			while (icol > maxcol)
 +			while (maxcol < icol + dp->hspans)
  				tbl->cols[++maxcol].spacing = SIZE_MAX;
  			col = tbl->cols + icol;
  			col->flags |= dp->layout->flags;

From: christos@zoulas.com (Christos Zoulas)
To: Ingo Schwarze <schwarze@usta.de>, gnats-bugs@NetBSD.org
Cc: Thomas Klausner <wiz@NetBSD.org>, Jason Thorpe <horpej@me.com>
Subject: Re: bin/54069
Date: Fri, 29 Mar 2019 17:59:32 -0400

 On Mar 29, 10:46pm, schwarze@usta.de (Ingo Schwarze) wrote:
 -- Subject: Re: bin/54069

 | Hi,
 | 
 | thanks to Jason for finding and reporting the bug, to Thomas for
 | making me aware of the PR, and to Christos for his commit, which
 | helped me understanding the bug.
 | 
 | I just fixed it upstream:
 | 
 |   http://mandoc.bsd.lv/cgi-bin/cvsweb/out.c#rev1.78
 | 
 | Note that my patch is smaller than Christos' and at a different
 | place.  It is easier to understand because that is where maxcol is
 | originally calculated.  Changing it later when it has already been
 | used for various purposes is somewhat confusing.   Maybe you want
 | to merge the upstream patch, to ease future updates and to avoid
 | ultimately ending up with a double fix.
 | 
 | Thanks,
 |   Ingo
 | 
 | 
 | Log Message:
 | -----------
 | Set the maximum column index in a tbl(7) to the maximum *right* edge 
 | of any cell span, not to the maximum *left* edge, which may be smaller 
 | if the last column of the table is only reached by horizontal spans,
 | but not by any regular cell in any row of the table.
 | Otherwise, the algorithm calculating column widths accessed memomy
 | after the end of the colwidth[] array, while it was trying to handle 
 | the rightmost column(s).
 | 
 | Crash reported by Jason Thorpe <thorpej at NetBSD>
 | via https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=54069
 | and via Thomas Klausner (wiz@).
 | Christos@ Zoulas sent a (correct, but slightly confusing) patch.
 | The patch i'm committing here is easier to understand.

 Thank you! I've committed your change.

 christos

State-Changed-From-To: open->closed
State-Changed-By: bsiegert@NetBSD.org
State-Changed-When: Mon, 08 Apr 2019 18:31:11 +0000
State-Changed-Why:
Fix committed.


>Unformatted:

Home	PR Database Search