Bug 80732

Does your patch correlate with bin/23562?

http://www.freebsd.org/cgi/query-pr.cgi?pr=bin/23562

-- 
Maxim Konovalov

On 5/7/05, Maxim Konovalov <maxim@macomnet.ru> wrote:
> Does your patch correlate with bin/23562?
>=20
> http://www.freebsd.org/cgi/query-pr.cgi?pr=3Dbin/23562

The bin/23562 is about same bugs of getty(8) and telnetd(8). It have
very old patch for telnetd(8) and no patch for getty(8).

The history of the patch in bin/23562 is mysterious. Originally it was
introduced in bin/6492 and then commited, many years ago:
http://www.freebsd.org/cgi/cvsweb.cgi/src/libexec/telnetd/Attic/telnetd.c.d=
iff?r1=3D1.7.2.4&r2=3D1.7.2.5
Three years later (August 2001) it was removed during: "Feature
merging and diff reduction between this code and crypto telnet":
http://www.freebsd.org/cgi/cvsweb.cgi/src/libexec/telnetd/Attic/telnetd.c.d=
iff?r1=3D1.29&r2=3D1.30
But the removing was not complete. telnetd.c still includes
<sys/mman.h> that had been need for mmap() and munmap() and manual
page of telnetd(8) still states that 'if' option of gettytab is
supported. The patch in bin/23562 was proposed in February 2001, i.e.
before it was removed in revision 1.30.
In July 2003 the sources of telnetd(8) were moved to
src/contrib/telnet where the revision numbering is different.

Personally I think my patch ot telnetd.c is better and the the
inclusion of <sys/mman.h> could be also removed from the telnetd.c as
needless.

This is an updated version of the telnetd(8) patch. It solves two more problems:
1. if a lexically cultivated or original contents of /etc/issue file
or 'im' parameter of /etc/gettytab is longer than BUFSIZ - 1 (1023
bytes) then telnetd will crash
2. if the last byte of the /etc/issue file or the 'im' parameter of
/etc/gettytab is a '%' char then telnetd will jump over the
null-terminator of the string.

The patch is attached to this email.

P.S. Original patch of getty(8) (of the main.c file) is still good and
desirable.

In my previous email the patch has been encoded by base64 by Gmail.
I'm sending the patch again after renaming it to *.txt file. I hope
this time Gmail will not use base64 encoding.

Added to the audit trail.

diff -ur telnetd.orig/ext.h /usr/src/contrib/telnet/telnetd/ext.h
--- telnetd.orig/ext.h	Mon May  9 16:41:45 2005
+++ /usr/src/contrib/telnet/telnetd/ext.h	Mon May  9 17:01:30 2005
@@ -112,8 +112,7 @@
 #endif
 	process_slc(unsigned char, unsigned char, cc_t),
 	ptyflush(void),
-	putchr(int),
-	putf(char *, char *),
+	putf(char *, char *, size_t),
 	recv_ayt(void),
 	send_do(int, int),
 	send_dont(int, int),
diff -ur telnetd.orig/telnetd.c /usr/src/contrib/telnet/telnetd/telnetd.c
--- telnetd.orig/telnetd.c	Mon May  9 16:42:13 2005
+++ /usr/src/contrib/telnet/telnetd/telnetd.c	Mon May  9 17:01:45 2005
@@ -42,7 +42,6 @@
 #include "telnetd.h"
 #include "pathnames.h"

-#include <sys/mman.h>
 #include <err.h>
 #include <libutil.h>
 #include <paths.h>
@@ -740,6 +739,7 @@
 	char *HE;
 	char *HN;
 	char *IM;
+	char *IF;
 	int nfd;

 	/*
@@ -900,22 +900,40 @@
 	 */

 	if (getent(defent, "default") == 1) {
-		char *cp=defstrs;
+		char *cp = defstrs;

 		HE = Getstr("he", &cp);
 		HN = Getstr("hn", &cp);
-		IM = Getstr("im", &cp);
-		if (HN && *HN)
+		IF = Getstr("if", &cp);
+		if (HN != NULL && *HN != 0)
 			(void) strlcpy(host_name, HN, sizeof(host_name));
-		if (IM == 0)
-			IM = strdup("");
+		if (IF != NULL) {
+			int if_fd;
+
+			if ((if_fd = open(IF, O_RDONLY)) != -1) {
+				struct stat if_fst;
+
+				fstat(if_fd, &if_fst);
+				IM = malloc(if_fst.st_size + 1);
+				read(if_fd, IM, if_fst.st_size);
+				IM[if_fst.st_size] = 0;
+				close(if_fd);
+			} else {
+				IF = NULL;
+			}
+		}
+		if (IF == NULL) {
+			IM = Getstr("im", &cp);
+			if (IM == NULL)
+				IM = strdup("");
+		}
 	} else {
 		IM = strdup(DEFAULT_IM);
-		HE = 0;
+		HE = NULL;
 	}
 	edithost(HE, host_name);
 	if (hostinfo && *IM)
-		putf(IM, ptyibuf2);
+		putf(IM, ptyibuf2, BUFSIZ);

 	if (pcc)
 		(void) strncat(ptyibuf2, ptyip, pcc+1);
diff -ur telnetd.orig/utility.c /usr/src/contrib/telnet/telnetd/utility.c
--- telnetd.orig/utility.c	Mon May  9 16:42:22 2005
+++ /usr/src/contrib/telnet/telnetd/utility.c	Mon May  9 17:01:38 2005
@@ -66,11 +66,9 @@
  * also flush the pty input buffer (by dropping its data) if it becomes
  * too full.
  */
-
-    void
-ttloop()
+void
+ttloop(void)
 {
-
     DIAG(TD_REPORT, output_data("td: ttloop\r\n"));
     if (nfrontp - nbackp > 0) {
 	netflush();
@@ -393,22 +391,6 @@
 	editedhost[sizeof editedhost - 1] = '\0';
 }

-static char *putlocation;
-
-static void
-putstr(const char *s)
-{
-
-	while (*s)
-		putchr(*s++);
-}
-
-void
-putchr(int cc)
-{
-	*putlocation++ = cc;
-}
-
 #ifdef __FreeBSD__
 static char fmtstr[] = { "%+" };
 #else
@@ -416,11 +398,12 @@
 #endif

 void
-putf(char *cp, char *where)
+putf(char *cp, char *where, size_t where_size)
 {
 	char *slash;
 	time_t t;
 	char db[100];
+	char ch_str[2] = {0, 0};
 #ifdef __FreeBSD__
 	static struct utsname kerninfo;

@@ -428,19 +411,13 @@
 		uname(&kerninfo);
 #endif

-	putlocation = where;
-
-	while (*cp) {
-		if (*cp =='\n') {
-			putstr("\r\n");
-			cp++;
-			continue;
+	while (*cp != 0) {
+		if (*cp == '\n') {
+			strlcat(where, "\r\n", where_size);
 		} else if (*cp != '%') {
-			putchr(*cp++);
-			continue;
-		}
-		switch (*++cp) {
-
+			*ch_str = *cp;
+			strlcat(where, ch_str, where_size);
+		} else switch (*++cp) {
 		case 't':
 #ifdef	STREAMSPTY
 			/* names are like /dev/pts/2 -- we want pts/2 */
@@ -448,46 +425,42 @@
 #else
 			slash = strrchr(line, '/');
 #endif
-			if (slash == (char *) 0)
-				putstr(line);
+			if (slash == NULL)
+				strlcat(where, line, where_size);
 			else
-				putstr(&slash[1]);
+				strlcat(where, &slash[1], where_size);
 			break;
-
 		case 'h':
-			putstr(editedhost);
+			strlcat(where, editedhost, where_size);
 			break;
-
 		case 'd':
 #ifdef __FreeBSD__
 			setlocale(LC_TIME, "");
 #endif
 			(void)time(&t);
 			(void)strftime(db, sizeof(db), fmtstr, localtime(&t));
-			putstr(db);
+			strlcat(where, db, where_size);
 			break;
-
 #ifdef __FreeBSD__
 		case 's':
-			putstr(kerninfo.sysname);
+			strlcat(where, kerninfo.sysname, where_size);
 			break;
-
 		case 'm':
-			putstr(kerninfo.machine);
+			strlcat(where, kerninfo.machine, where_size);
 			break;
-
 		case 'r':
-			putstr(kerninfo.release);
+			strlcat(where, kerninfo.release, where_size);
 			break;
-
 		case 'v':
-			putstr(kerninfo.version);
+			strlcat(where, kerninfo.version, where_size);
 			break;
 #endif
-
 		case '%':
-			putchr('%');
+			*ch_str = '%';
+			strlcat(where, ch_str, where_size);
 			break;
+		case 0:
+			return;
 		}
 		cp++;
 	}

Why didn't you check the return code from malloc(3) and read(2)?
Could you please fix that?  Could you remove all style(9) changes
from your diff?

-- 
Maxim Konovalov

Maxim Konovalov wrote:
> Why didn't you check the return code from malloc(3) and read(2)?
> Could you please fix that?  Could you remove all style(9) changes
> from your diff?

Fixed. Thank you for the comment. New diff of telnetd(8) is attached to 
this email (inline).

P.S. The original diff of getty(8) doesn't need any change so far.

During a private discussion with Maxim Konovalov I've realized that the 
original getty(8) actually does print the IF file (/etc/issue) but only 
once - before the very first IM message. When I did make the getty(8) 
and the telnetd(8) patches I thought that the IM is really an initial 
message and there should be no super-initial message before it; i.e. if 
the IF file is defined and accessible, its contents should be a 
replacement of the IM.

I still think so, but also I think getty(8) and telnetd(8) should behave 
by the same way during a login process. Try doing following experiment 
with the original getty(8) on any pseudo-terminal:

1. Press the Enter key a few times. After each pressing you'll see the 
IM message before each "login:" prompt.
2. Try login with a wrong password or a wrong username and after you get 
a new "login:" prompt press the Enter key a few times again. Now you'll 
see only the "login:" prompts, without the IM message before any of them.

If you do the same experiment with the original telnetd(8) you see the 
IM message only once - before the very first "login:" prompt.

Therefore, in addition to the last version of telnetd(8) patch, I made a 
new version of getty(8) patch that makes getty(8) to be with the same 
behavior like telnetd(8).

Comment 9 Eitan Adler 2017-12-31 08:00:49 UTC

For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped

Comment 10 Graham Perrin 2022-10-17 12:40:59 UTC

Keyword: 

    patch
or  patch-ready

– in lieu of summary line prefix: 

    [patch]

* bulk change for the keyword
* summary lines may be edited manually (not in bulk). 

Keyword descriptions and search interface: 

    <https://bugs.freebsd.org/bugzilla/describekeywords.cgi>