Bug 195436 - patch utility, line number overflows checks
Summary: patch utility, line number overflows checks
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Pedro F. Giffuni
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-27 01:05 UTC by David CARLIER
Modified: 2014-12-17 00:59 UTC (History)
2 users (show)

See Also:

Attachments
Proposed patch (6.24 KB, patch)
2014-11-27 01:05 UTC, David CARLIER 		no flags Details | Diff
Proposed patch (5.88 KB, patch)
2014-11-27 15:23 UTC, David CARLIER 		no flags Details | Diff
Proposed patch (6.38 KB, patch)
2014-11-29 09:11 UTC, David CARLIER 		no flags Details | Diff
Patch based on OpenBSD (6.12 KB, patch)
2014-12-07 19:42 UTC, Pedro F. Giffuni 		no flags Details | Diff
Patch based on OpenBSD (6.08 KB, patch)
2014-12-07 21:00 UTC, Pedro F. Giffuni 		no flags Details | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David CARLIER 2014-11-27 01:05:41 UTC 
Created attachment 149926 [details]
Proposed patch

Inspired by OpenBSD's version. it does some overflow checks in case line numbers are beyond of LONG_MAX limits and such.
Comment 1 Pedro F. Giffuni freebsd_committer freebsd_triage 2014-11-27 11:22:09 UTC 
Hello and thank you;

the patch looks good, however I am traveling at this time and I can't play with it.
Comment 2 David CARLIER 2014-11-27 15:23:19 UTC 
Created attachment 149941 [details]
Proposed patch
Comment 3 Pedro F. Giffuni freebsd_committer freebsd_triage 2014-11-29 07:54:16 UTC 
If the changes come from OpenBSD, it would be good to update the CVS revision information as the tags are used for reference in future updates.
Comment 4 David CARLIER 2014-11-29 09:11:24 UTC 
Created attachment 149982 [details]
Proposed patch
Comment 5 Pedro F. Giffuni freebsd_committer freebsd_triage 2014-12-05 14:39:27 UTC 
Grab this.
Comment 6 Pedro F. Giffuni freebsd_committer freebsd_triage 2014-12-07 19:42:03 UTC 
Created attachment 150316 [details]
Patch based on OpenBSD

I am inclined to follow the complete OpenBSD change.

This revision precedes your original change:

Introduce strtolinenum to properly check line numbers while parsing:
no signs, no spaces, just digits, 0 <= x <= LONG_MAX
Comment 7 Pedro F. Giffuni freebsd_committer freebsd_triage 2014-12-07 21:00:03 UTC 
Created attachment 150320 [details]
Patch based on OpenBSD

Update the patch, now that I have merged some previous changes from OpenBSD.
Comment 8 commit-hook freebsd_committer freebsd_triage 2014-12-08 15:10:59 UTC 
A commit references this bug:

Author: pfg
Date: Mon Dec  8 15:10:49 UTC 2014
New revision: 275612
URL: https://svnweb.freebsd.org/changeset/base/275612

Log:
  patch(1): avoid line number overflows

  Introduce strtolinenum to properly check line numbers while parsing:
  no signs, no spaces, just digits, 0 <= x <= LONG_MAX

  Properly validate line ranges supplied in diff file to prevent overflows.
  Also fixes an out of boundary memory access because the resulting values
  are used as array indices.

  PR:	195436
  Obtained from:	OpenBSD (CVS pch.c rev 1.45, 1,46, common.h rev 1.28)
  MFC after:	1 week

Changes:
  head/usr.bin/patch/common.h
  head/usr.bin/patch/pch.c
Comment 9 Pedro F. Giffuni freebsd_committer freebsd_triage 2014-12-08 15:16:00 UTC 
Thank you for pointing out the issue.

I went with the OpenBSD patch as it makes it easier to keep in sync (we have been diverging in other ways though).
I didn't update the OpenBSD revision number on the affected files because we are skipping at least one change: not sure if we want to drop the SCCS support just yet (legacy ports exist).
Comment 10 commit-hook freebsd_committer freebsd_triage 2014-12-16 21:14:33 UTC 
A commit references this bug:

Author: pfg
Date: Tue Dec 16 21:13:56 UTC 2014
New revision: 275841
URL: https://svnweb.freebsd.org/changeset/base/275841

Log:
  MFC	r275553, r275612;

  patch(1): Bring fixes from OpenBSD

  Check fstat return value.  Use off_t for file size and offsets.
  Avoid iterating over end of string.

  Introduce strtolinenum to properly check line numbers while parsing:
  no signs, no spaces, just digits, 0 <= x <= LONG_MAX

  Properly validate line ranges supplied in diff file to prevent overflows.
  Also fixes an out of boundary memory access because the resulting values
  are used as array indices.

  PR:		195436
  Obtained from:	OpenBSD

Changes:
_U  stable/10/
  stable/10/usr.bin/patch/common.h
  stable/10/usr.bin/patch/pch.c
Comment 11 Pedro F. Giffuni freebsd_committer freebsd_triage 2014-12-17 00:59:58 UTC 
OpenBSD has new changes that will have to be analyzed but this issue can be closed now.