Re: [perl #42594] [PATCH] Probable buffer overflow in compilers/imcc/parser_util.c

2007-04-18 Thread Mehmet Yavuz Selim Soyturk

+format[sizeof(format - 1)] = '\0';



Shouldn't that be 'format[sizeof(format) - 1]' ?

--
Mehmet


Re: [perl #42594] [PATCH] Probable buffer overflow in compilers/imcc/parser_util.c

2007-04-18 Thread Steve Peters
On Wed, Apr 18, 2007 at 11:18:20AM +0200, Mehmet Yavuz Selim Soyturk wrote:
 +format[sizeof(format - 1)] = '\0';
 
 
 Shouldn't that be 'format[sizeof(format) - 1]' ?
 

Yes, thanks!  Good catch!

Steve


Re: [perl #42594] [PATCH] Probable buffer overflow in compilers/imcc/parser_util.c

2007-04-18 Thread Leopold Toetsch
Am Dienstag, 17. April 2007 23:48 schrieb Steve Peters:
 +        strncpy(format, fmt, sizeof(format - 1));

Me thinks, that it's better to check the len of the format as it grows, 
considering the amount what would be strcat'ed. If an index really would 
overflow format, a proper syntax error could be emitted.

leo


[perl #42594] [PATCH] Probable buffer overflow in compilers/imcc/parser_util.c

2007-04-17 Thread via RT
# New Ticket Created by  Steve Peters 
# Please include the string:  [perl #42594]
# in the subject line of all future correspondence about this issue. 
# URL: http://rt.perl.org/rt3/Ticket/Display.html?id=42594 


I don't know how easily this is reached, but since the fmt variable
is only NULL checked, it seems like this would be possible to reached.

Steve Peters
[EMAIL PROTECTED]

Index: compilers/imcc/parser_util.c
===
--- compilers/imcc/parser_util.c(revision 18270)
+++ compilers/imcc/parser_util.c(working copy)
@@ -494,8 +494,10 @@
 if (len = 2)
 len -= 2;
 format[len] = '\0';
-if (fmt  *fmt)
-strcpy(format, fmt);
+if (fmt  *fmt) {
+strncpy(format, fmt, sizeof(format - 1));
+format[sizeof(format - 1)] = '\0';
+}
 #if 1
 IMCC_debug(interp, DEBUG_PARSER,%s %s\t%s\n, name, format, fullname);
 #endif



Re: [perl #42594] [PATCH] Probable buffer overflow in compilers/imcc/parser_util.c

2007-04-17 Thread chromatic
On Tuesday 17 April 2007 14:48, Steve Peters wrote:

 I don't know how easily this is reached, but since the fmt variable
 is only NULL checked, it seems like this would be possible to reached.

Hm, this patch breaks some tests for me:

t/compilers/imcc/imcpasm/optc.t1   256431  8
t/compilers/imcc/imcpasm/sub.t 2   512 22  1-2

It looks like it's eating the first source register of certain opcodes.  
Changing (format - 1) to (format) in both patched lines fixes it for me, but 
I'm not confident enough in the patch after that to check it in.  Here's the 
verbose output.

-- c

t/compilers/imcc/imcpasm/optc.t 
not ok 8 - in P param
# Failed test (t/compilers/imcc/imcpasm/optc.t at line 226)
#   '# IMCC does produce b0rken PASM files
# # see http://[EMAIL PROTECTED]/rt3/Ticket/Display.html?id=32392
# _main:
#  new P0,
#  set P0, 42
# @pcc_sub_call_0:
#  set_args
#  set_p_pc P1, foo
#  get_results
#  invokecc P1
#  noop
#  end
# foo:
#  get_params
#  print P0
#  set_returns
#  returncc
# '
# doesn't match '/_main:
#  new (P\d+), \d+ # \.Undef
#  set \1, 42
# @pcc_sub_call_\d:
#  set_args
#  set_p_pc (P\d+), foo
#  get_results
#  invokecc \2
#  noop
#  end
# foo:
#  get_params
#  print P0
#  set_returns
#  returncc/
# '

t/compilers/imcc/imcpasm/sub.t
not ok 1 - non-constant dest bsr, invoke
# Failed test (t/compilers/imcc/imcpasm/sub.t at line 11)
#   '# IMCC does produce b0rken PASM files
# # see http://[EMAIL PROTECTED]/rt3/Ticket/Display.html?id=32392
# _main:
#  new P0,
#  set_addr I0, _sub1
#  set P0, I0
#  invokecc P0
#  ret
# _sub1:
#  ret
# '
# doesn't match '/^# IMCC does produce b0rken PASM files
# # see http://[EMAIL PROTECTED]/rt3/Ticket/Display.html\?id=32392
# _main:
#  new P(\d+), \d+ # \.Sub
#  set_addr I(\d+), _sub1
#  set P\1, I\2
#  invokecc P\1
#  ret
# _sub1:
#  ret/
# '
not ok 2 - nonlocal bsr
# Failed test (t/compilers/imcc/imcpasm/sub.t at line 34)
#   '# IMCC does produce b0rken PASM files
# # see http://[EMAIL PROTECTED]/rt3/Ticket/Display.html?id=32392
# _main:
#  new P0,
#  set_addr I0, _f
#  set P0, I0
#  invokecc P0
#  ret
# _f:
#  ret
# '
# doesn't match '/^# IMCC does produce b0rken PASM files
# # see http://[EMAIL PROTECTED]/rt3/Ticket/Display.html\?id=32392
# _main:
#  new P(\d+), \d+ # \.Sub
#  set_addr I(\d+), _f
#  set P\1, I\2
#  invokecc P\1
#  ret
# _f:
#  ret/
# '