Patches item #1930733, was opened at 2008-03-31 21:02 Message generated for change (Comment added) made by eighthave You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=478072&aid=1930733...

Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: puredata Group: bugfix Status: Pending Resolution: Fixed Priority: 7 Private: No Submitted By: Thomas Grill (xovo) Assigned to: Miller Puckette (millerpuckette) Summary: 0.41-4: prevent buffer overrun in m_class.c

Initial Comment: the array only has MAXPDARG elements.......

--- m_class.ori.c 2008-04-01 03:00:09.000000000 +0200 +++ m_class.c 2008-04-01 03:00:12.000000000 +0200 @@ -763,7 +763,7 @@ va_start(ap, fmt); while (1) { - if (nargs > MAXPDARG) + if (nargs >= MAXPDARG) { pd_error(x, "pd_vmess: only %d allowed", MAXPDARG); break;

----------------------------------------------------------------------

Comment By: Hans-Christoph Steiner (eighthave)

Date: 2009-01-12 15:39

Message: it seems that MAXPDARG was just replaced with 10. Seems kind of arbitrary, anyone know why 10? Here's the commit:

http://pure-data.svn.sourceforge.net/viewvc/pure-data/trunk/pd/src/m_class.c...

----------------------------------------------------------------------

Comment By: IOhannes m zmlnig (zmoelnig) Date: 2009-01-12 11:34

Message: fixed in 0.42

----------------------------------------------------------------------

Comment By: Miller Puckette (millerpuckette) Date: 2008-05-26 12:46

Message: Logged In: YES user_id=313747 Originator: NO

It will work fine to allow MAXARG+1 arguments (6) to vmess; the MAXARGS restriction in typedmess() is unrelated. I think that's the best way since apparently some code exists that depends on MAXARG+1. It's not clear what the maximum should really be; since this is a convenience routine anyway, I'm thinking it should be expanded, say to 10 or so.

----------------------------------------------------------------------

Comment By: Hans-Christoph Steiner (eighthave) Date: 2008-05-26 11:57

Message: Logged In: YES user_id=27104 Originator: NO

It seems to me that the logic of the function needs to change, but adding +1 to the array will prevent the overflow, though it's not a very clean way to do it.

Works for now, I suppose.

----------------------------------------------------------------------

Comment By: IOhannes m zmlnig (zmoelnig) Date: 2008-05-26 03:32

Message: Logged In: YES user_id=564396 Originator: NO

as thomas has pointed out, there _is_ a buffer overrun which should be fixed rather than merely unapplying the patch because it gives weird error messages.

so for the ease of applying it, here is the patch that fixes the leak by adding an additional element to the array (i'll apply it to pd-ext 0.40)

Index: m_class.c =================================================================== --- m_class.c (Revision 9919) +++ m_class.c (Arbeitskopie) @@ -772,7 +772,7 @@ void pd_vmess(t_pd *x, t_symbol *sel, char *fmt, ...) { va_list ap; - t_atom arg[MAXPDARG], *at =arg; + t_atom arg[MAXPDARG+1], *at =arg; int nargs = 0; char *fp = fmt;

----------------------------------------------------------------------

Comment By: Thomas Grill (xovo) Date: 2008-05-26 03:17

Message: Logged In: YES user_id=350252 Originator: YES

Is the arg array too small then and must have MAXPDARGS+1 elements instead?

In the loop, the "at" pointer subsequently points to the elements of "arg". If the switch statement is reached with a value of "nargs == MAXPDARG", then "arg" points to a position outside the array.

----------------------------------------------------------------------

Comment By: Hans-Christoph Steiner (eighthave) Date: 2008-05-25 17:52

Message: Logged In: YES user_id=27104 Originator: NO

This patch causes this message when clicking on abstractions and subpatches in run mode:

error: pd_vmess: only 5 allowed ... you might be able to track this down from the Find menu.

I think this patch might be invalid since nargs is only used to send to typedmess() as argc, in which case 5 is the appropriate value.

----------------------------------------------------------------------

Comment By: Hans-Christoph Steiner (eighthave) Date: 2008-04-15 19:42

Message: Logged In: YES user_id=27104 Originator: NO

checked into branches/pd-extended/0.40

----------------------------------------------------------------------

You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=478072&aid=1930733...