strncpy crusade - libcob part I

cobc:
* common.c (check_current_date): improved validation
* common.c, intrinsic.c: favor strcpy, snprintf over strncpy

cobc/cobc.c

@@ -5214,17 +5214,10 @@ set_picture (struct cb_field *field, char *picture, size_t picture_len)
 		strcpy (picture, "X ANY LENGTH");
 		return 1;
 	} else {
-		size_t fpic_len;
 		if (!field->pic) {
 			return 0;
 		}
-		fpic_len = strlen (field->pic->orig);
-		if (fpic_len < picture_len) {
-			memcpy (picture, field->pic->orig, fpic_len + 1);
-		} else {
-			memcpy (picture, field->pic->orig, picture_len - 1);
-			picture [picture_len] = 0;
-		}
+		snprintf (picture, picture_len, "%s", field->pic->orig);
 		return 1;
 	}
 }

config/runtime.cfg

@@ -168,7 +168,7 @@
 #   Parameter name:  current_date
 #          Purpose:  specify an alternate Date/Time to be returned to ACCEPT
 #                    clauses this is used for testing purposes or to tweak
-#                    a missing offset partial setting is allowed
+#                    a missing offset; partial setting is allowed
 #             Type:  numeric string in format YYYYDDMMHH24MISS or date string
 #          Default:  the operating system date is used
 #          Example:  COB_CURRENT_DATE "2016/03/16 16:40:52"

libcob/ChangeLog

@@ -1,4 +1,9 @@
 
+2022-10-05  Simon Sobisch <[email protected]>
+
+	* common.c (check_current_date): improved validation
+	* common.c, intrinsic.c: favor strcpy, snprintf over strncpy
+
 2022-10-04  Simon Sobisch <[email protected]>
 
 	* common.c (cob_stack_trace_internal, output_procedure_stack_entry):

libcob/common.c

@@ -304,7 +304,7 @@ static unsigned int	conf_runtime_error_displayed = 0;
 static unsigned int	last_runtime_error_line = 0;
 static const char	*last_runtime_error_file = NULL;
 
-static char			runtime_err_str[COB_ERRBUF_SIZE] = {0};	/* emergency buffer */
+static char			runtime_err_str[COB_ERRBUF_SIZE] = { 0 };	/* emergency buffer */
 static int			exit_code = 0;	/* exit code when leaving libcob */
 
 #ifndef COB_WITHOUT_JMP
@@ -797,14 +797,23 @@ cob_get_source_line ()
 static char *
 cob_get_strerror (void)
 {
-	char * msg;
-	msg = cob_cache_malloc ((size_t)COB_ERRBUF_SIZE);
+	size_t size;
+	char *ret;
 #ifdef HAVE_STRERROR
-	strncpy (msg, strerror (errno), COB_ERRBUF_SIZE - 1);
+	char *msg = strerror (errno);
+	size = strlen (msg);
 #else
-	snprintf (msg, COB_ERRBUF_SIZE - 1, _("system error %d"), errno);
+	char msg[COB_ERRBUF_SIZE];
+	size = snprintf (msg, COB_ERRBUF_SIZE, _("system error %d"), errno);
+	if (size >= COB_ERRBUF_SIZE) {
+		/* very unlikely, but if that would be a bad msg catalog
+		   we don't want to memcpy from invalid data */
+		size = COB_ERRBUF_SIZE - 1;
+	}
 #endif
-	return msg;
+	ret = cob_cache_malloc (size + 1);
+	memcpy (ret, msg, size + 1);
+	return ret;
 }
 
 #ifdef	HAVE_SIGNAL_H
@@ -2380,7 +2389,7 @@ cob_field_to_string (const cob_field *f, void *str, const size_t maxsize)
 	size_t		i;
 
 	if (unlikely (f == NULL)) {
-		strncpy (str, _("NULL field"), maxsize);
+		snprintf (str, maxsize, "%s", ("NULL field"));
 		return;
 	}
 
@@ -2390,7 +2399,7 @@ cob_field_to_string (const cob_field *f, void *str, const size_t maxsize)
 	}
 	/* check if field has data assigned (may be a BASED / LINKAGE item) */
 	if (unlikely (f->data == NULL)) {
-		strncpy (str, _("field with NULL address"), maxsize);
+		snprintf (str, maxsize, "%s", ("field with NULL address"));
 		return;
 	}
 	for (i = f->size - 1; ; i--) {
@@ -4188,22 +4197,23 @@ check_current_date ()
 		iso_timezone[0] = 'Z';
 	} else if (cobsetptr->cob_date[j] == '+'
 	        || cobsetptr->cob_date[j] == '-') {
-		char *iso_timezone_ptr = (char *)&iso_timezone;
-		strncpy (iso_timezone_ptr, cobsetptr->cob_date + j, 6);
-		iso_timezone[6] = 0;	/* just to keep the analyzer happy */
-		if (strlen (iso_timezone_ptr) == 3) {
-			strcpy (iso_timezone_ptr + 3, "00");
-		} else if (iso_timezone[3] == ':') {
-			strncpy (iso_timezone_ptr + 3, cobsetptr->cob_date + j + 4, 3);
-		}
-		for (i=1; iso_timezone[i] != 0; i++) {
+		int len = snprintf (&iso_timezone[0], 6, "%s", cobsetptr->cob_date + j);
+		if (len == 3) {
+			memcpy (iso_timezone + 3, "00", 3);
+		} else
+		if (len >= 5 && iso_timezone[3] == ':') {
+			snprintf (&iso_timezone[3], 3, cobsetptr->cob_date + j + 4);
+			len--;
+		}
+		if (len > 5) {
+			ret = 1;
+		}
+		for (i=1; i < 5 && iso_timezone[i] != 0; i++) {
 			if (!isdigit ((unsigned char)iso_timezone[i])) {
 				break;
 			}
-			if (++i == 4) {
-				break;
-			}
 		}
+		i--;
 		if (i == 4) {
 			offset = COB_D2I (iso_timezone[1]) * 60 * 10
 				+ COB_D2I (iso_timezone[2]) * 60
@@ -7654,7 +7664,7 @@ cob_runtime_error (const char *fmt, ...)
 			if (more_error_procedures) {
 				/* fresh error buffer with guaranteed size */
 				char local_err_str[COB_ERRBUF_SIZE];
-				memcpy (&local_err_str, runtime_err_str, COB_ERRBUF_SIZE);
+				memcpy (local_err_str, runtime_err_str, COB_ERRBUF_SIZE);
 
 				/* ensure that error handlers set their own locations */
 				cob_source_file = NULL;

libcob/intrinsic.c

@@ -2531,40 +2531,44 @@ copy_data_to_null_terminated_str (cob_field *f, char * const out_str,
 	out_str[length] = '\0';
 }
 
-static void
+static int
 split_around_t (const char *str, char *first, char *second)
 {
-	int i;
+	int i, ret = 0;
 	size_t first_length;
 	size_t second_length;
 
 	/* Find 'T' */
 	for (i = 0; str[i] != '\0' && str[i] != 'T'; ++i);
 
 	/* Copy everything before 'T' into first (if present) */
-	if (i < COB_DATESTR_MAX) {
-		first_length = i;
-	} else {
+	if (i > COB_DATESTR_MAX) {
 		first_length = COB_DATESTR_MAX;
+		ret = COB_DATESTR_MAX + 1;
+	} else {
+		first_length = i;
 	}
 	if (first != NULL) {
-		strncpy (first, str, first_length);
-		first[first_length] = '\0';
+		/* possible overflow checked above,
+		   snprintf ensures terminated buffer */
+		snprintf (first, first_length + 1, "%s", str);
 	}
 
 	/* If there is anything after 'T', copy it into second (if present) */
 	if (second != NULL) {
-		if (strlen (str) - i == 0) {
+		str += i + 1;
+		second_length = strlen (str);
+		if (second_length == 0) {
 			second[0] = '\0';
 		} else {
-			second_length = strlen (str) - i - 1U;
 			if (second_length > COB_TIMESTR_MAX) {
 				second_length = COB_TIMESTR_MAX;
+				ret = COB_TIMESTR_MAX + 1 + i;
 			}
-			strncpy (second, str + i + 1U, second_length);
-			second[second_length] = '\0';
+			snprintf (second, second_length + 1, "%s", str);
 		}
 	}
+	return ret;
 }
 
 static int
@@ -3606,7 +3610,9 @@ cob_valid_datetime_format (const char *format, const char decimal_point)
 	struct date_format	date_format;
 	struct time_format	time_format;
 
-	split_around_t (format, date_format_str, time_format_str);
+	if (split_around_t (format, date_format_str, time_format_str)) {
+		return 0;
+	}
 
 	if (!cob_valid_date_format (date_format_str)
 	 || !cob_valid_time_format (time_format_str, decimal_point)) {
@@ -6700,7 +6706,9 @@ cob_intr_formatted_datetime (const int offset, const int length,
 		goto invalid_args;
 	}
 
-	split_around_t (fmt_str, date_fmt_str, time_fmt_str);
+	if (split_around_t (fmt_str, date_fmt_str, time_fmt_str)) {
+		goto invalid_args;
+	}
 
 	time_fmt = parse_time_format_string (time_fmt_str);
 	if (use_system_offset) {
@@ -6776,24 +6784,26 @@ cob_intr_test_formatted_datetime (cob_field *format_field,
 		goto invalid_args;
 	}
 
-	/* Move date/time to respective variables */
+	/* Move date/time to respective variables;
+	   note: all fields and sizes were validated above */
 	if (date_present && time_present) {
-		split_around_t (datetime_format_str, date_format_str, time_format_str);
+		split_around_t (datetime_format_str,
+			date_format_str, time_format_str);
 	} else if (date_present) {
-		strncpy (date_format_str, datetime_format_str, COB_DATESTR_MAX);
+		strcpy (date_format_str, datetime_format_str);
 	} else { /* time_present */
-		strncpy (time_format_str, datetime_format_str, COB_TIMESTR_MAX);
+		strcpy (time_format_str, datetime_format_str);
 	}
 
+	/* Move format fields respective variables;
+	   note: all fields and sizes were validated during compile */
 	if (date_present && time_present) {
 		split_around_t (formatted_datetime, formatted_date, formatted_time);
 	} else if (date_present) {
-		strncpy (formatted_date, formatted_datetime, COB_DATESTR_MAX);
+		strcpy (formatted_date, formatted_datetime);
 	} else { /* time_present */
-		strncpy (formatted_time, formatted_datetime, COB_TIMESTR_MAX);
+		strcpy (formatted_time, formatted_datetime);
 	}
-	/* silence warnings */
-	formatted_date[COB_DATESTR_MAX] = formatted_time[COB_TIMESTR_MAX] = 0;
 
 	/* Set time offset */
 	if (date_present) {

libcob/numeric.c

@@ -778,11 +778,10 @@ cob_decimal_set_double (cob_decimal *d, const double v)
 static double
 cob_decimal_get_double (cob_decimal *d)
 {
-	double		v;
+	double		v = 0.0;
 	cob_sli_t	n;
 
 	cob_not_finite = 0;
-	v = 0.0;
 	if (unlikely (mpz_size (d->value) == 0)) {
 		return v;
 	}

libcob/strings.c

@@ -434,9 +434,9 @@ inspect_common (cob_field *f1, cob_field *f2, const enum inspect_type type)
 /* an INSPECT is split into multiple parts:
    one-time cob_inspect_init   (setting up memory and markers)
    multiple:
-	cob_inspect_start         (setting inspect_start/end)
-	cob_inspect_before        (optional, adjusting inspect_end)
-	cob_inspect_after         (optional, adjusting inspect_start)
+	cob_inspect_start          (setting inspect_start/end)
+	cob_inspect_before         (optional, adjusting inspect_end)
+	cob_inspect_after          (optional, adjusting inspect_start)
    one of:
 	cob_inspect_characters/cob_inspect_converting (until 3.2)/cob_inspect_all/
 	cob_inspect_leading/cob_inspect_trailing/cob_inspect_first

tests/testsuite.src/run_accept.at

@@ -1,4 +1,4 @@
-## Copyright (C) 2003-2012, 2014-2017, 2020 Free Software Foundation, Inc.
+## Copyright (C) 2003-2012, 2014-2017, 2020, 2022 Free Software Foundation, Inc.
 ## Written by Keisuke Nishida, Roger While, Simon Sobisch
 ##
 ## This file is part of GnuCOBOL.
@@ -48,7 +48,7 @@ AT_CLEANUP
 #
 
 AT_SETUP([ACCEPT FROM TIME / DATE / DAY / DAY-OF-WEEK (1)])
-AT_KEYWORDS([INSPECT CONVERTING])
+AT_KEYWORDS([INSPECT CONVERTING configuration COB_CURRENT_DATE])
 
 AT_DATA([prog.cob], [
        IDENTIFICATION   DIVISION.
@@ -111,6 +111,29 @@ AT_DATA([prog.cob], [
 AT_CHECK([$COMPILE prog.cob], [0], [], [])
 AT_CHECK([$COBCRUN_DIRECT ./prog], [0], [], [])
 
+# testing all variants for the offset (to not break anything hard,
+# actual testing of the values need a bigger test in COBOL)
+AT_CHECK([COB_CURRENT_DATE="YYYYMMDDHHMMSS+01:00" \
+$COBCRUN_DIRECT ./prog], [0], [], [])
+AT_CHECK([COB_CURRENT_DATE="YYYYMMDDHHMMSS-0115" \
+$COBCRUN_DIRECT ./prog], [0], [], [])
+AT_CHECK([COB_CURRENT_DATE="YYYYMMDDHHMMSS+02" \
+$COBCRUN_DIRECT ./prog], [0], [], [])
+AT_CHECK([COB_CURRENT_DATE="YYYYMMDDHHMMSSZ" \
+$COBCRUN_DIRECT ./prog], [0], [], [])
+AT_CHECK([COB_CURRENT_DATE="YYYYMMDDHHMMSS-011569" \
+$COBCRUN_DIRECT ./prog], [0], [],
+[libcob: warning: COB_CURRENT_DATE 'YYYYMMDDHHMMSS-011569' is invalid
+])
+AT_CHECK([COB_CURRENT_DATE="YYYYMMDDHHMMSS+01:0000" \
+$COBCRUN_DIRECT ./prog], [0], [],
+[libcob: warning: COB_CURRENT_DATE 'YYYYMMDDHHMMSS+01:0000' is invalid
+])
+AT_CHECK([COB_CURRENT_DATE="YYYYMMDDHHMMSS-010A" \
+$COBCRUN_DIRECT ./prog], [0], [],
+[libcob: warning: COB_CURRENT_DATE 'YYYYMMDDHHMMSS-010A' is invalid
+])
+
 AT_CLEANUP