NetBSD Problem Report #50746

From www@NetBSD.org  Tue Feb  2 10:29:05 2016
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 "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id D14BB7AC9C
	for <gnats-bugs@gnats.NetBSD.org>; Tue,  2 Feb 2016 10:29:05 +0000 (UTC)
Message-Id: <20160202102905.009AA7ACB5@mollari.NetBSD.org>
Date: Tue,  2 Feb 2016 10:29:04 +0000 (UTC)
From: dcb314@hotmail.com
Reply-To: dcb314@hotmail.com
To: gnats-bugs@NetBSD.org
Subject: src/usr.bin/mail/edit.c:166]: (style) Array index 'i' is used before limits check.
X-Send-Pr-Version: www-1.0

>Number:         50746
>Category:       bin
>Synopsis:       src/usr.bin/mail/edit.c:166]: (style) Array index 'i' is used before limits check.
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Feb 02 10:30:00 +0000 2016
>Closed-Date:    Wed Feb 03 08:18:11 +0000 2016
>Last-Modified:  Wed Feb 03 11:20:00 +0000 2016
>Originator:     David Binderman
>Release:        cvs dated 20160131
>Organization:
>Environment:
>Description:
Source code is

    for (i = 0; msgvec[i] && i < msgCount; i++) {


Maybe something like

  for (i = 0; (i < msgCount) && (msgvec[i] != 0); i++) {


might be better.

>How-To-Repeat:

>Fix:

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/50746 CVS commit: src/usr.bin/mail
Date: Wed, 3 Feb 2016 00:18:58 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Wed Feb  3 05:18:58 UTC 2016

 Modified Files:
 	src/usr.bin/mail: edit.c

 Log Message:
 PR/50746: David Binderman: Check bounds before dereferencing.


 To generate a diff of this commit:
 cvs rdiff -u -r1.27 -r1.28 src/usr.bin/mail/edit.c

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

State-Changed-From-To: open->closed
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Wed, 03 Feb 2016 08:18:11 +0000
State-Changed-Why:
Fixed by christos, thanks!


From: Steffen Nurpmeso <steffen@sdaoden.eu>
To: gnats-bugs@NetBSD.org
Cc: dcb314@hotmail.com, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
        Steffen (Daode) Nurpmeso <steffen@sdaoden.eu>
Subject: Re: PR/50746 CVS commit: src/usr.bin/mail
Date: Wed, 03 Feb 2016 12:18:45 +0100

 "Christos Zoulas" <christos@netbsd.org> wrote:
  |The following reply was made to PR bin/50746; it has been noted by GNATS.
  |
  |From: "Christos Zoulas" <christos@netbsd.org>
  |To: gnats-bugs@gnats.NetBSD.org
  |Cc: 
  |Subject: PR/50746 CVS commit: src/usr.bin/mail
  |Date: Wed, 3 Feb 2016 00:18:58 -0500
  |
  | Module Name: src
  | Committed By:        christos
  | Date:                Wed Feb  3 05:18:58 UTC 2016
  | 
  | Modified Files:
  |  src/usr.bin/mail: edit.c
  | 
  | Log Message:
  | PR/50746: David Binderman: Check bounds before dereferencing.

 That is unnecessary since you will not get there if the argument
 vector is empty, and if you get there it'll be 0 terminated.
 So in a sense the original code is more true.  In fact the check
 against msgCount is what is really fishy here.
 That change seems to originate in DragonFly, the original code
 was already in 2BSD, and i wildly guess that there was a time when
 it wasn't clear how things would proceed, but already then
 getmsglist() ensured the 0 termination.

 --steffen

>Unformatted:

Home	PR Database Search