[RFC PATCH 1/1] patch: fix seg fault caused by malformed patch

Jim Keniston jkenisto at linux.vnet.ibm.com
Thu Mar 5 23:58:45 UTC 2015


The busybox FAQ says it's time to post a polite reminder about my
suggested patch.  See my original mail below.

Here's a sample patch that triggers the seg fault:
=== cut here ===
-- dwarves.orig	2015-02-25 01:45:27.753000000 +0000
+++ dwarves	2015-02-25 01:46:08.199000000 +0000
@@ -1,7 +1,7 @@
 Bashful
 Doc
 Dopey
-Grouchy
+Grumpy
 Happy
 Sleepy
 Sneezy
=== cut here ===

BTW, if you don't mind spending ~20 bytes of rodata for potentially
clearer error messages, you could do
+			if (!oldname)
+				oldname = xstrdup("MISSING_FILENAME");
+			else if (!newname)
+				newname = xstrdup("MISSING_FILENAME");
instead.

Jim Keniston

On Thu, 2015-02-26 at 15:10 -0800, Jim Keniston wrote:
> busybox's patch command dies with a SIGSEGV if the "---" line that heralds
> a new file to patch is missing.  GNU patch silently does what's apparently
> wanted, based on the "+++" line.
> 
> Signed-off-by: Jim Keniston <jkenisto at linux.vnet.ibm.com>
> 
> diff -up ./editors/patch.c.orig ./editors/patch.c
> --- ./editors/patch.c.orig	2015-02-25 18:28:29.231096889 -0600
> +++ ./editors/patch.c	2015-02-26 16:21:23.560131205 -0600
> @@ -345,6 +345,8 @@ done:
>  // state 1: Found +++ file indicator, look for @@
>  // state 2: In hunk: counting initial context lines
>  // state 3: In hunk: getting body
> +// Like GNU patch, we don't require a --- line before the +++, and
> +// also allow the --- after the +++ line.
> 
>  int patch_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
>  int patch_main(int argc UNUSED_PARAM, char **argv)
> @@ -462,6 +464,14 @@ int patch_main(int argc UNUSED_PARAM, ch
>  			TT.context = 0;
>  			state = 2;
> 
> +			// If the --- line is missing or malformed, either oldname
> +			// or (for -R) newname could be NULL -- but not both.  Like
> +			// GNU patch, proceed based on the +++ line, and avoid SEGVs.
> +			if (!oldname)
> +				oldname = xstrdup(newname);
> +			else if (!newname)
> +				newname = xstrdup(oldname);
> +
>  			// If this is the first hunk, open the file.
>  			if (TT.filein == -1) {
>  				int oldsum, newsum, empty = 0;
> 
> 
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
> 




More information about the busybox mailing list