Hi All,

I came across this segfault accidentally while live coding today.  When I ran:

prints strtol("077")

I got a crash.  The issue I see is that we have OENTRY's for prints
(and printks) that allow taking in a number:

  { "printks.i",S(PRINTKS),WR, 3,   "",   "iiM",
    (SUBR)printksset,(SUBR)printks, NULL },L },
  { "prints.i",S(PRINTS),0,   1,   "",   "iM",   (SUBR)printsset, NULL, NULL },

I did not test printks, but at least for prints, this caused a crash.

I think both of these seem wrong and should not have entries. I'm
guessing fprintks.i in Opcodes/fout.c is okay as the first arg is a
file and that can be a number?

Could someone else take a look and confirm whether these can be removed?

Thanks!

Hi All,

I came across this segfault accidentally while live coding today.  When I ran:

prints strtol("077")

I got a crash.  The issue I see is that we have OENTRY's for prints
(and printks) that allow taking in a number:

 { "printks.i",S(PRINTKS),WR, 3,   "",   "iiM",
   (SUBR)printksset,(SUBR)printks, NULL },L },
 { "prints.i",S(PRINTS),0,   1,   "",   "iM",   (SUBR)printsset, NULL, NULL },

I did not test printks, but at least for prints, this caused a crash.

I think both of these seem wrong and should not have entries. I'm
guessing fprintks.i in Opcodes/fout.c is okay as the first arg is a
file and that can be a number?

Could someone else take a look and confirm whether these can be removed?

Thanks!
steven

Hm, that's a Csound 7 thing to fix for sure to have opcodes inserted
that do type checks on pfields. (I've been thinking we need runtime
checks inserted anyways if we're going to have generic lists).

As it is now though, it was way too easy for me to use prints during
the live code session and cause a crash; I imagine it would take down
any API program as well and I could see it as easily being one of the
reason we get crashes.  (Someone debugs their orchestra, types prints
instead of print, crash, etc.).

I wrote a CSD test and added it to the test suite.  It should cause
Travis and AppVeyor to fail our builds until we put in a fix.  (I'll
take a look at prints when I get back home shortly, but we need to
cover all opcodes where this can happen.)




On Mon, Nov 6, 2017 at 3:36 PM, Victor Lazzarini  wrote:
> Those are for taking pfields I think. Many string-input opcodes have that.
>
> Victor Lazzarini
> Dean of Arts, Celtic Studies, and Philosophy
> Maynooth University
> Ireland
>
> On 6 Nov 2017, at 20:17, Steven Yi  wrote:
>
> Hi All,
>
> I came across this segfault accidentally while live coding today.  When I
> ran:
>
> prints strtol("077")
>
> I got a crash.  The issue I see is that we have OENTRY's for prints
> (and printks) that allow taking in a number:
>
>  { "printks.i",S(PRINTKS),WR, 3,   "",   "iiM",
>    (SUBR)printksset,(SUBR)printks, NULL },L },
>  { "prints.i",S(PRINTS),0,   1,   "",   "iM",   (SUBR)printsset, NULL, NULL
> },
>
> I did not test printks, but at least for prints, this caused a crash.
>
> I think both of these seem wrong and should not have entries. I'm
> guessing fprintks.i in Opcodes/fout.c is okay as the first arg is a
> file and that can be a number?
>
> Could someone else take a look and confirm whether these can be removed?
>
> Thanks!

Hm, that's a Csound 7 thing to fix for sure to have opcodes inserted
that do type checks on pfields. (I've been thinking we need runtime
checks inserted anyways if we're going to have generic lists).

As it is now though, it was way too easy for me to use prints during
the live code session and cause a crash; I imagine it would take down
any API program as well and I could see it as easily being one of the
reason we get crashes.  (Someone debugs their orchestra, types prints
instead of print, crash, etc.).

I wrote a CSD test and added it to the test suite.  It should cause
Travis and AppVeyor to fail our builds until we put in a fix.  (I'll
take a look at prints when I get back home shortly, but we need to
cover all opcodes where this can happen.)




On Mon, Nov 6, 2017 at 3:36 PM, Victor Lazzarini <Victor.Lazzarini@mu.ie> wrote:

Those are for taking pfields I think. Many string-input opcodes have that.

Victor Lazzarini

Dean of Arts, Celtic Studies, and Philosophy

Maynooth University

Ireland

On 6 Nov 2017, at 20:17, Steven Yi <stevenyi@GMAIL.COM> wrote:

Hi All,

I came across this segfault accidentally while live coding today.  When I

ran:

prints strtol("077")

I got a crash.  The issue I see is that we have OENTRY's for prints

(and printks) that allow taking in a number:

{ "printks.i",S(PRINTKS),WR, 3,   "",   "iiM",

  (SUBR)printksset,(SUBR)printks, NULL },L },

{ "prints.i",S(PRINTS),0,   1,   "",   "iM",   (SUBR)printsset, NULL, NULL

},

I did not test printks, but at least for prints, this caused a crash.

I think both of these seem wrong and should not have entries. I'm

guessing fprintks.i in Opcodes/fout.c is okay as the first arg is a

file and that can be a number?

Could someone else take a look and confirm whether these can be removed?

Thanks!

steven

I put in some defensive code for prints to check results of
get_arg_string before trying to dereference. It's made the CSD test
pass here and just return an error rather than crash.  I looked at
other uses of get_arg_string (pasted below) and it looks to me that
most of these are are used where expecting either a string filename or
number.  It'd be good though to get another pair of eyes to look at
this.

Status Code File Line Column Project Read/Write
Confirmed reference strncpy(pvfilnam,get_arg_string(csound,
*p->ifilno), MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\Opcodes\vpvoc.c 203 26
csound64-static Unknown
Confirmed reference strncpy(name,get_arg_string(csound,
*p->iFileCode), 1023);
c:\Users\stevenyi\work\csound\csound\OOps\diskin2.c 331 22
csound64-static Unknown
Confirmed reference strncpy(name,get_arg_string(csound, *ifilcod),
1023); c:\Users\stevenyi\work\csound\csound\OOps\diskin2.c 1051 22
csound64-static Unknown
Confirmed reference strncpy(name,get_arg_string(csound,
*p->iFileCode), 1023);
c:\Users\stevenyi\work\csound\csound\OOps\diskin2.c 1575 22
csound64-static Unknown
Confirmed reference strncpy(soundoname, get_arg_string(csound,
*p->ifilcod), 1023); c:\Users\stevenyi\work\csound\csound\OOps\dumpf.c
78 27 csound64-static Unknown
Confirmed reference strncpy(soundoname, get_arg_string(csound,
*p->ifilcod), 1023); c:\Users\stevenyi\work\csound\csound\OOps\dumpf.c
131 27 csound64-static Unknown
Confirmed reference strncpy(soundoname, get_arg_string(csound,
*p->ifilcod), 1023); c:\Users\stevenyi\work\csound\csound\OOps\dumpf.c
186 27 csound64-static Unknown
Confirmed reference strncpy(soundoname, get_arg_string(csound,
*p->ifilcod), 1023); c:\Users\stevenyi\work\csound\csound\OOps\dumpf.c
238 27 csound64-static Unknown
Confirmed reference strncpy(soundiname, get_arg_string(csound,
*p->ifilcod), 1023); c:\Users\stevenyi\work\csound\csound\OOps\dumpf.c
387 28 csound64-static Unknown
Confirmed reference strncpy(soundiname, get_arg_string(csound,
*p->ifilcod), 1023); c:\Users\stevenyi\work\csound\csound\OOps\dumpf.c
467 27 csound64-static Unknown
Confirmed reference strncpy(soundiname, get_arg_string(csound,
*p->ifilcod), 1023); c:\Users\stevenyi\work\csound\csound\OOps\dumpf.c
521 27 csound64-static Unknown
Confirmed reference strncpy(soundiname, get_arg_string(csound,
*p->ifilcod), 1023); c:\Users\stevenyi\work\csound\csound\OOps\dumpf.c
575 27 csound64-static Unknown
Confirmed reference strncpy(soundiname, get_arg_string(csound,
*p->ifilcod), 1023); c:\Users\stevenyi\work\csound\csound\OOps\dumpf.c
775 27 csound64-static Unknown
Confirmed reference name = cs_strdup(csound, get_arg_string(csound,
*iFile)); c:\Users\stevenyi\work\csound\csound\Opcodes\fout.c 97 32
csound64-static Unknown
Confirmed reference fname = cs_strdup(csound, get_arg_string(csound,
*p->iFile)); c:\Users\stevenyi\work\csound\csound\Opcodes\fout.c 659
35 csound64-static Unknown
Confirmed reference else strncpy(filename,
get_arg_string(csound,*p->ifilno), MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\Opcodes\ftgen.c 279 30
csound64-static Unknown
Confirmed reference else strncpy(filename,
get_arg_string(csound,*p->ifilno), MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\Opcodes\ftgen.c 492 30
csound64-static Unknown
Confirmed reference p1 = (MYFLT) strarg2insno(csound,
get_arg_string(csound, *p->kInsNo), 1);
c:\Users\stevenyi\work\csound\csound\OOps\goto_ops.c 197 41
csound64-static Unknown
Confirmed reference n = (int) strarg2opcno(csound,
get_arg_string(csound,*p->r), 1,
c:\Users\stevenyi\work\csound\csound\Engine\insert.c 2348 40
csound64-static Unknown
Confirmed reference get_arg_string(csound, *p->args[1]), 1);
c:\Users\stevenyi\work\csound\csound\Engine\linevent.c 413 44
csound64-static Unknown
Confirmed reference get_arg_string(csound, *p->args[1]), 1);
c:\Users\stevenyi\work\csound\csound\Engine\linevent.c 480 44
csound64-static Unknown
Confirmed reference get_arg_string(csound, *p->args[0]), 1);
c:\Users\stevenyi\work\csound\csound\Engine\linevent.c 535 44
csound64-static Unknown
Confirmed reference fname = csound->Strdup(csound,
get_arg_string(csound, *p->Sfname));
c:\Users\stevenyi\work\csound\csound\Opcodes\loscilx.c 48 40
csound64-static Unknown
Confirmed reference (char*) get_arg_string(csound, *p->ifn),
c:\Users\stevenyi\work\csound\csound\Opcodes\loscilx.c 196 42
csound64-static Unknown
Confirmed reference strncpy(name,get_arg_string(csound,
*p->iFileCode), 1023);
c:\Users\stevenyi\work\csound\csound\Opcodes\mp3in.c 105 22
csound64-static Unknown
Confirmed reference strncpy(name,get_arg_string(csound,
*p->iFileCode), 1023);
c:\Users\stevenyi\work\csound\csound\Opcodes\mp3in.c 282 22
csound64-static Unknown
Confirmed reference char *ss = get_arg_string(csound,*p->insno);
c:\Users\stevenyi\work\csound\csound\Engine\musmon.c 537 18
csound64-static Unknown
Confirmed reference char  *s2 = get_arg_string(csound, *((MYFLT*)p));
c:\Users\stevenyi\work\csound\csound\Engine\namedins.c 179 19
csound64-static Unknown
Confirmed reference cs_strdup(csound, get_arg_string(csound,
csound->currevent->p[n+start]));
c:\Users\stevenyi\work\csound\csound\Opcodes\nlfilt.c 245 29
csound64-static Unknown
Confirmed reference char *ss = get_arg_string(csound,*p->ins);
c:\Users\stevenyi\work\csound\csound\Opcodes\pitch0.c 38 18
csound64-static Unknown
Confirmed reference char *ss = get_arg_string(csound,*p->ins);
c:\Users\stevenyi\work\csound\csound\Opcodes\pitch0.c 76 18
csound64-static Unknown
Confirmed reference char *ss = get_arg_string(csound,*p->instrnum);
c:\Users\stevenyi\work\csound\csound\Opcodes\pitch0.c 139 18
csound64-static Unknown
Confirmed reference char *ss = get_arg_string(csound,*p->instrnum);
c:\Users\stevenyi\work\csound\csound\Opcodes\pitch0.c 165 18
csound64-static Unknown
Confirmed reference strncpy(pvfilnam, get_arg_string(csound,
*p->ifilno), MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\OOps\pstream.c 384 25
csound64-static Unknown
Confirmed reference strncpy(pvfilnam,get_arg_string(csound,
*p->ifilno), MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\Opcodes\pvadd.c 85 26
csound64-static Unknown
Confirmed reference strncpy(pvfilnam,get_arg_string(csound,
*p->ifilno), MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\Opcodes\pvinterp.c 63 26
csound64-static Unknown
Confirmed reference strncpy(pvfilnam,get_arg_string(csound,
*p->ifilno), MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\Opcodes\pvinterp.c 180 26
csound64-static Unknown
Confirmed reference strncpy(pvfilnam,get_arg_string(csound,
*p->ifilno), MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\Opcodes\pvinterp.c 371 26
csound64-static Unknown
Confirmed reference strncpy(pvfilnam,get_arg_string(csound,
*p->ifilno), MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\Opcodes\pvread.c 77 26
csound64-static Unknown
Confirmed reference strncpy(fname,get_arg_string(csound, *p->file),
MAXNAME-1); c:\Users\stevenyi\work\csound\csound\Opcodes\pvsbasic.c
218 21 csound64-static Unknown
Confirmed reference strncpy(fname,get_arg_string(csound, *p->file),
MAXNAME-1); c:\Users\stevenyi\work\csound\csound\Opcodes\pvsbasic.c
355 21 csound64-static Unknown
Confirmed reference char* get_arg_string(CSOUND *csound, MYFLT p)
c:\Users\stevenyi\work\csound\csound\Engine\rdscor.c 28 7
csound64-static Unknown
Confirmed reference char *ss = get_arg_string(csound, *p->args[0]);
c:\Users\stevenyi\work\csound\csound\OOps\schedule.c 399 18
csound64-static Unknown
Confirmed reference unquote(name, get_arg_string(csound, *p->args[0]),
512); c:\Users\stevenyi\work\csound\csound\OOps\schedule.c 469 21
csound64-static Unknown
Confirmed reference fname = csound->Strdup(csound,
get_arg_string(csound,*p->fname));
c:\Users\stevenyi\work\csound\csound\Opcodes\sfont.c 160 40
csound64-static Unknown
Confirmed reference strncpy(soundiname, get_arg_string(csound,
*p->ifilno), 1023);
c:\Users\stevenyi\work\csound\csound\OOps\sndinfUG.c 48 27
csound64-static Unknown
Confirmed reference strncpy(soundiname, get_arg_string(csound,
*p->ifilno), 1023);
c:\Users\stevenyi\work\csound\csound\OOps\sndinfUG.c 325 27
csound64-static Unknown
Confirmed reference strncpy(soundiname, get_arg_string(csound,
*p->ifilno), 1023);
c:\Users\stevenyi\work\csound\csound\OOps\sndinfUG.c 347 27
csound64-static Unknown
Confirmed reference ss = get_arg_string(csound, *p->indx);
c:\Users\stevenyi\work\csound\csound\OOps\str_ops.c 134 12
csound64-static Unknown
Confirmed reference ss = get_arg_string(csound, *p->indx);
c:\Users\stevenyi\work\csound\csound\OOps\str_ops.c 250 12
csound64-static Unknown
Confirmed reference s = get_arg_string(csound, *p->str);
c:\Users\stevenyi\work\csound\csound\OOps\str_ops.c 620 11
csound64-static Unknown
Confirmed reference s = get_arg_string(csound, *p->str);
c:\Users\stevenyi\work\csound\csound\OOps\str_ops.c 714 13
csound64-static Unknown
Confirmed reference strncpy(filnam, get_arg_string(csound,
*p->ifilcod), MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\OOps\ugens3.c 828 23
csound64-static Unknown
Confirmed reference strncpy(lpfilname, get_arg_string(csound,
*p->ifilcod), MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\OOps\ugens5.c 619 26
csound64-static Unknown
Confirmed reference strncpy(pvfilnam,get_arg_string(csound,
*p->ifilno), MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\Opcodes\ugens8.c 53 26
csound64-static Unknown
Confirmed reference strncpy(cvfilnam,get_arg_string(csound,
*p->ifilno), MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\Opcodes\ugens9.c 46 26
csound64-static Unknown
Confirmed reference strncpy(IRfile.sfname,get_arg_string(csound,
*p->ifilno), 511);
c:\Users\stevenyi\work\csound\csound\Opcodes\ugens9.c 397 31
csound64-static Unknown
Confirmed reference strncpy(fname,get_arg_string(csound,
*((MYFLT*)name_arg)),MAXNAME-1);
c:\Users\stevenyi\work\csound\csound\Opcodes\ugnorman.c 121 23
csound64-static Unknown
Confirmed reference char* arg_string = get_arg_string(csound,
*p->ifilcod); c:\Users\stevenyi\work\csound\csound\OOps\ugrw1.c 962 24
csound64-static Unknown



On Mon, Nov 6, 2017 at 4:03 PM, Victor Lazzarini  wrote:
> yep, we should try to defend against that.
>
> Victor Lazzarini
> Dean of Arts, Celtic Studies, and Philosophy
> Maynooth University
> Ireland
>
> On 6 Nov 2017, at 20:59, Steven Yi  wrote:
>
> Hm, that's a Csound 7 thing to fix for sure to have opcodes inserted
> that do type checks on pfields. (I've been thinking we need runtime
> checks inserted anyways if we're going to have generic lists).
>
> As it is now though, it was way too easy for me to use prints during
> the live code session and cause a crash; I imagine it would take down
> any API program as well and I could see it as easily being one of the
> reason we get crashes.  (Someone debugs their orchestra, types prints
> instead of print, crash, etc.).
>
> I wrote a CSD test and added it to the test suite.  It should cause
> Travis and AppVeyor to fail our builds until we put in a fix.  (I'll
> take a look at prints when I get back home shortly, but we need to
> cover all opcodes where this can happen.)
>
>
>
>
> On Mon, Nov 6, 2017 at 3:36 PM, Victor Lazzarini 
> wrote:
>
> Those are for taking pfields I think. Many string-input opcodes have that.
>
>
> Victor Lazzarini
>
> Dean of Arts, Celtic Studies, and Philosophy
>
> Maynooth University
>
> Ireland
>
>
> On 6 Nov 2017, at 20:17, Steven Yi  wrote:
>
>
> Hi All,
>
>
> I came across this segfault accidentally while live coding today.  When I
>
> ran:
>
>
> prints strtol("077")
>
>
> I got a crash.  The issue I see is that we have OENTRY's for prints
>
> (and printks) that allow taking in a number:
>
>
> { "printks.i",S(PRINTKS),WR, 3,   "",   "iiM",
>
>   (SUBR)printksset,(SUBR)printks, NULL },L },
>
> { "prints.i",S(PRINTS),0,   1,   "",   "iM",   (SUBR)printsset, NULL, NULL
>
> },
>
>
> I did not test printks, but at least for prints, this caused a crash.
>
>
> I think both of these seem wrong and should not have entries. I'm
>
> guessing fprintks.i in Opcodes/fout.c is okay as the first arg is a
>
> file and that can be a number?
>
>
> Could someone else take a look and confirm whether these can be removed?
>
>
> Thanks!
>