From: Denis Fondras <denis@openbsd.org>
Subject: Re: pppd(8): cleanup
To: Theo Buehler <tb@theobuehler.org>
Cc: Denis Fondras <denis@openbsd.org>, tech@openbsd.org
Date: Sat, 17 Aug 2024 15:22:57 +0200

Le Sat, Aug 17, 2024 at 11:26:38AM +0200, Theo Buehler a écrit :
> On Sat, Aug 17, 2024 at 11:21:28AM +0200, Denis Fondras wrote:
> >  	case 'r':
> >  	    if (report_fp != NULL)
> >  		fclose (report_fp);
> > -	    report_file = copy_of (optarg);
> > +	    report_file = strdup(optarg);
> 
> so you need an error check here
> 

Thank you Theo. Here is an updated diff.

Index: fsm.c
===================================================================
RCS file: /cvs/src/usr.sbin/pppd/fsm.c,v
diff -u -p -r1.9 fsm.c
--- fsm.c	9 Aug 2024 05:16:13 -0000	1.9
+++ fsm.c	17 Aug 2024 13:20:22 -0000
@@ -399,6 +399,7 @@ fsm_rconfreq(fsm *f, int id, u_char *inp
 	if( f->callbacks->down )
 	    (*f->callbacks->down)(f);	/* Inform upper layers */
 	fsm_sconfreq(f, 0);		/* Send initial Configure-Request */
+	f->state = REQSENT;
 	break;
 
     case STOPPED:
@@ -434,10 +435,10 @@ fsm_rconfreq(fsm *f, int id, u_char *inp
 	f->nakloops = 0;
 
     } else {
-	/* we sent CONFACK or CONFREJ */
+	/* we sent CONFNACK or CONFREJ */
 	if (f->state != ACKRCVD)
 	    f->state = REQSENT;
-	if( code == CONFNAK )
+	if(code == CONFNAK)
 	    ++f->nakloops;
     }
 }
@@ -539,6 +540,7 @@ fsm_rconfnakrej(fsm *f, int code, int id
 	    f->state = STOPPED;		/* kludge for stopping CCP */
 	else
 	    fsm_sconfreq(f, 0);		/* Send Configure-Request */
+	    f->state = REQSENT;
 	break;
 
     case ACKRCVD:
Index: chat/chat.c
===================================================================
RCS file: /cvs/src/usr.sbin/pppd/chat/chat.c,v
diff -u -p -r1.37 chat.c
--- chat/chat.c	10 Aug 2024 05:32:28 -0000	1.37
+++ chat/chat.c	17 Aug 2024 13:20:22 -0000
@@ -172,7 +172,6 @@ int clear_report_next = 0;
 
 int say_next = 0, hup_next = 0;
 
-void *dup_mem(void *b, size_t c);
 void usage(void);
 void logmsg(const char *fmt, ...);
 void fatal(int code, const char *fmt, ...);
@@ -204,22 +203,6 @@ int vfmtmsg(char *, int, const char *, v
 
 int main(int, char *[]);
 
-void *
-dup_mem(void *b, size_t c)
-{
-    void *ans = malloc (c);
-    if (!ans)
-	fatal(2, "memory error!");
-
-    memcpy (ans, b, c);
-    return ans;
-}
-
-void *copy_of (char *s)
-{
-    return dup_mem (s, strlen (s) + 1);
-}
-
 /*
  * chat [ -v ] [-T number] [-U number] [ -t timeout ] [ -f chat-file ] \
  * [ -r report-file ] \
@@ -257,7 +240,8 @@ main(int argc, char **argv)
 	    break;
 
 	case 'f':
-	    chat_file = strdup(optarg);
+	    if ((chat_file = strdup(optarg)) == NULL)
+		fatal(2, "chat_file memory error!");
 	    break;
 
 	case 't':
@@ -267,7 +251,8 @@ main(int argc, char **argv)
 	case 'r':
 	    if (report_fp != NULL)
 		fclose (report_fp);
-	    report_file = copy_of (optarg);
+	    if ((report_file = strdup(optarg)) == NULL)
+		fatal(2, "report_file memory error!");
 	    report_fp   = fopen (report_file, "a");
 	    if (report_fp != NULL) {
 		if (verbose)
@@ -278,11 +263,14 @@ main(int argc, char **argv)
 	    break;
 
 	case 'T':
-	    phone_num = strdup(optarg);
+	    if ((phone_num = strdup(optarg)) == NULL)
+		fatal(2, "phone_num memory error!");
 	    break;
 
 	case 'U':
 	    phone_num2 = strdup(optarg);
+	    if ((phone_num2 = strdup(optarg)) == NULL)
+		fatal(2, "phone_num2 memory error!");
 	    break;
 
 	case ':':