Rergession in readcmd (commit 55c46b7)

Rergession in readcmd (commit 55c46b7)

[Alexey Gladkov]

Greeting!

readcmd put trash in the variables.

Test case:

$ cat /tmp/conf
Include common.conf
FullSpeedCPU yes

$ cat /tmp/z.sh
while read option params; do
  printf '[%s] [%s]\n' "$option" "$params"
done < /tmp/conf

Result:

$ dash /tmp/z.sh
[Include] [common.conf]
[FullSpeedCPU] [yesFnf]

-- 
Rgrds, legion

Re: Rergession in readcmd (commit 55c46b7)

[Herbert Xu]

Alexey Gladkov <gladkov.alexey@gmail.com> wrote:
> Greeting!
> 
> readcmd put trash in the variables.
> 
> Test case:
> 
> $ cat /tmp/conf
> Include common.conf
> FullSpeedCPU yes
> 
> $ cat /tmp/z.sh
> while read option params; do
>  printf '[%s] [%s]\n' "$option" "$params"
> done < /tmp/conf
> 
> Result:
> 
> $ dash /tmp/z.sh
> [Include] [common.conf]
> [FullSpeedCPU] [yesFnf]

Thanks for the report.  It's an off-by-one error causing the
NUL termination to disappear.

diff --git a/ChangeLog b/ChangeLog
index 9d397e4..13c9010 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2009-08-31  Herbert Xu <herbert@gondor.apana.org.au>
+
+	* Fix NUL termination in readcmd.
+
 2009-08-11  Herbert Xu <herbert@gondor.apana.org.au>
 
 	* Pass EV_TESTED into evalcmd.
diff --git a/src/miscbltin.c b/src/miscbltin.c
index cca0f6c..be746b2 100644
--- a/src/miscbltin.c
+++ b/src/miscbltin.c
@@ -182,7 +182,7 @@ resetbs:
 		backslash = 0;
 	}
 	STACKSTRNUL(p);
-	readcmd_handle_line(stackblock(), ap, p - (char *)stackblock());
+	readcmd_handle_line(stackblock(), ap, p + 1 - (char *)stackblock());
 	return status;
 }
 
Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Rergession in readcmd (commit 55c46b7)

[Alexey Gladkov]

31.08.2009 14:12, Herbert Xu wrote:
>> $ dash /tmp/z.sh
>> [Include] [common.conf]
>> [FullSpeedCPU] [yesFnf]
> 
> Thanks for the report.  It's an off-by-one error causing the
> NUL termination to disappear.

I found another example:

$ tr -d '[:print:]' < /etc/passwd |tr -d '\t\n' |wc -c
0

$ dash -c 'while read o p; do printf "[%s] [%s]\n" "$o" "$p"; done <
/etc/passwd' |tr -d '[:print:]' |tr -d '[:space:]' |wc -c
61

bug is not fixed yet :(

-- 
Rgrds, legion

Re: Rergession in readcmd (commit 55c46b7)

[Herbert Xu]

Alexey Gladkov <gladkov.alexey@gmail.com> wrote:
> 
> I found another example:
> 
> $ tr -d '[:print:]' < /etc/passwd |tr -d '\t\n' |wc -c
> 0
> 
> $ dash -c 'while read o p; do printf "[%s] [%s]\n" "$o" "$p"; done <
> /etc/passwd' |tr -d '[:print:]' |tr -d '[:space:]' |wc -c
> 61
> 
> bug is not fixed yet :(

This bug is caused by an off-by-one error in the recordregion
call in readcmd.  It included the terminating NUL in the region
which causes ifsbreakup to include the string after it for scanning.

Setting the correct length fixes the problem.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/ChangeLog b/ChangeLog
index fabb0e1..7be8e86 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2009-11-26  Herbert Xu <herbert@gondor.apana.org.au>
+
+	Fix off-by-one recordregion in readcmd.
+
 2009-09-28  Jim Meyering  <meyering@redhat.com>
 
 	don't read-uninitialized for \177 in a here-doc
diff --git a/src/miscbltin.c b/src/miscbltin.c
index ec9872d..046f2f2 100644
--- a/src/miscbltin.c
+++ b/src/miscbltin.c
@@ -84,7 +84,7 @@ readcmd_handle_line(char *line, char **ap, size_t len)
 	backup = sstrdup(line);
 
 	arglist.lastp = &arglist.list;
-	recordregion(0, len, 0);
+	recordregion(0, len - 1, 0);
 	
 	ifsbreakup(s, &arglist);
 	*arglist.lastp = NULL;

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt