bin/53368: Potential integer overflow in usr.bin/patch/inp.c

[thomas.barabosch%fkie.fraunhofer.de@localhost]

>Number:         53368
>Category:       bin
>Synopsis:       Potential integer overflow in usr.bin/patch/inp.c
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Jun 15 16:55:00 +0000 2018
>Originator:     Thomas Barabosch
>Release:        7.1.2
>Organization:
Fraunhofer FKIE
>Environment:
>Description:
There is a potential integer overflow in usr.bin/patch/inp.c. In function reallocate_lines there is an array reallocation with realloc that utilizes a multiplication. It is better to use reallocarr.3 (at least for NetBSD > 7.0). 

I checked with other BSDs and found that this issue was patched in OpenBSD (check github mirror, commit c905b2617f2672181a099d8df3fa499ef3abdcf9).

I've drafted a patch. Reallocarr returns an int, not a pointer. Therefore, I checked for zero and threw away the assignment i_ptr = p since this function directly works on the array. Does this look OK? Or is there another way to fix this issue?
>How-To-Repeat:

>Fix:
--- usr.bin/patch/inp.c	2018-05-14 00:00:49.573719141 +0200
+++ usr.bin/patch/inp.c.patched	2018-06-15 18:43:30.914882168 +0200
@@ -122,8 +122,8 @@
 	size_t	new_size;
 
 	new_size = *lines_allocated * 3 / 2;
-	p = realloc(i_ptr, (new_size + 2) * sizeof(char *));
-	if (p == NULL) {	/* shucks, it was a near thing */
+	int res = reallocarr(&i_ptr, new_size + 2,  sizeof(char *));
+	if (res != 0) {	/* shucks, it was a near thing */
 		munmap(i_womp, i_size);
 		i_womp = NULL;
 		free(i_ptr);
@@ -132,7 +132,6 @@
 		return false;
 	}
 	*lines_allocated = new_size;
-	i_ptr = p;
 	return true;
 }