Disallow strings as ops in cmp-ok in Test.pm6

[p6rt]

Migrated from rt.perl.org#128283 (status was 'rejected')

Searchable as RT128283$

[p6rt]

From @zoffixznet

The operators for `cmp-ok` subroutine can be given as strings or as Callables. The problem with strings is the subroutine evals them as EVAL "&infix​:<$op>" and the current implementation makes it impossible to use, say '<' as an operator​:

  <Zoffix> m​: use Test; cmp-ok 2, '<', 5, 'seems sane';
  <camelia> rakudo-moar beb3c9​: OUTPUT«not ok 1 - seems sane␤␤# Failed test 'seems sane'␤# at /tmp/kqui3siS7u line 1␤# Could not use '<' as a comparator␤»

It does work with &[<] instead of '<' and considering that's only 1 character longer and there's no 100% clean way to allow any op in a string, I propose we ditch the string version entirely.

While I don't know anything about SETTINGS, the current implementation also has a security hole, at least in camelia where `run`, while restricted in general code, is given a free pass when injected inside the string comparator passed to cmp-ok​:

  <Zoffix> m​: run "ls"
  <camelia> rakudo-moar beb3c9​: OUTPUT«run is disallowed in restricted setting␤ in sub restricted at src/RESTRICTED.setting line 1␤ in sub run at src/RESTRICTED.setting line 14␤ in block <unit> at /tmp/wWcCMBi30n line 1␤␤»
  <Zoffix> m​: use Test; cmp-ok '', '~~>;warn run "ls"; <z', '', '';
  <camelia> rakudo-moar beb3c9​: OUTPUT«Perlito␤dalek-queue␤evalbot␤evalbot.log␤foo␤lib␤log␤mbox␤nqp-js␤p1␤p2␤p6eval-token␤perl5␤rakudo-j-1␤rakudo-j-2␤rakudo-j-inst␤rakudo-j-inst-1␤rakudo-j-inst-2␤rakudo-m-1␤rakudo-m-2␤rakudo-m-inst␤rakudo-m-inst-1␤rak…»

[p6rt]

From @smls

Sounds like throwing out the baby with the bath water, to me.

there's no 100% clean way to allow any op in a string

What makes you so sure?
EVAL is not the only way to this this, nor arguably the correct way.

For example, the following works​:

  my $name = "<";

  my &op = CORE​::.first(*.key eq "\&infix​:<$name>" | "\&infix​:«$name»").value;

  if &op {
  say op 5, 42; # True
  say op 42, 5; # False
  }

This should be immune to injection attacks too.

Using low-level NQP code to do this, might make it even more robust (i.e. avoid having to check both <$name> and «$name» against the string representation, and instead check for the routine name directly).

[p6rt]

The RT System itself - Status changed from 'new' to 'open'

[p6rt]

From @smls

Ok, admittedly using `.first` to index the PseudoStash is a little silly.
Something like this is probably saner​:

  my &op = CORE​::{"\&infix​:<$name>"} //
  CORE​::{"\&infix​:«$name»"};

[p6rt]

From @smls

Or this​:

  my &op = (try &​::("infix​:<$name>")) //
  (try &​::("infix​:«$name»"));

[p6rt]

From @zoffixznet

On Sun May 29 08​:51​:45 2016, smls75@​gmail.com wrote​:

What makes you so sure?

I asked the channel about this before and no one gave a satisfying answer. I go off my knowledge. If you know a way, state it; there's no need to attack my knowledge of the language.

Playing with the solutions you offered, I see that they they don't work with custom user-defined operators and likely break with operators containing both > and » in them.

I think psch++ came up with a working solution somewhere here, but there are too many variants for me to follow​: http://irclog.perlgeek.de/perl6-dev/2016-05-29#i_12568075

[p6rt]

From @smls

there's no need to attack my knowledge of the language

Sorry, wasn't meant as an attack.
It's just that you sounded very sure when you said there's no way to implement it cleanly, so I wondered if there had been a proper review leading to that conclusion.

Playing with the solutions you offered, I see that they they don't
work with custom user-defined operators and likely break with
operators containing both > and » in them.

The last one works with user-defined operators.

To make it also work with operators containing both > and », it can be extended as follows​:

  my &op = &​::("infix​:<$name>")
  // &​::("infix​:«$name»")
  // &​::("infix​:<{$name.subst(/<before <[<>]> >/, "\\", :g)}>")
  // die "Infix operator $name not found.";

[p6rt]

From @zoffixznet

The last one works with user-defined operators.

It works only when the custom operators are in scope. Is there a way to make it work inside Test.pm6 code?

  zoffix@​leliana​:/tmp/tmp.avxBKNwr4B$ cat foo.p6
  sub cmp ($name, $lhs, $rhs) {
  my &op = &​::("infix​:<$name>")
  // &​::("infix​:«$name»")
  // &​::("infix​:<{$name.subst(/<before <[<>]> >/, "\\", :g)}>")
  // die "Infix operator $name not found.";
  op($lhs, $rhs);
  }
 
  {
  sub infix​:<foo> { $^a &lt; $^b };
  say 2 foo 3;
  say cmp 'foo', 2, 3;
  }
 
  zoffix@​leliana​:/tmp/tmp.avxBKNwr4B$ perl6 foo.p6
  True
  Infix operator foo not found.
  in sub cmp at foo.p6 line 2
  in block <unit> at foo.p6 line 13

[p6rt]

From @zoffixznet

While wordy, this appears to work and removes all the issues raised in this ticket​:

  sub cmp ($name, $a, $b) {
  my &op = &​::("infix​:<$name>")
  // &​::("infix​:«$name»")
  // &​::("infix​:<$name.subst(/<before <[<>]> >/, "\\", :g)>")
  // &CALLER​::("infix​:<$name>")
  // &CALLER​::("infix​:«$name»")
  // &CALLER​::("infix​:<$name.subst(/<before <[<>]> >/, "\\", :g)>")
  // die "Infix operator $name not found.";

  op($a, $b);
  }

  {
  sub infix​:<< <» >> { $^a + $^b }
  sub infix​:<< «> >> { $^a + $^b }
  sub infix​:<< «» >> { $^a + $^b }
  say cmp '<', 3, 4;
  say cmp '>', 3, 4;
  say cmp '<»', 3, 4;
  say cmp '«>', 3, 4;
  say cmp '«»', 3, 4;
  }

I'll submit a PR and add extra tests for this to roast tonight or tomorrow.

[p6rt]

From @smls

Unfortunately, `CALLER​::` only contains symbols that were lexically declared directly in the caller's scope, and not those inherited from its parent scopes.

I'm not sure if a proper run-time lookup of a lexical symbol as seen from the caller scope is possible. Maybe there's some NQP trickery to do it? Ask jnthn, he is the expert on scopes and callframes...

[p6rt]

From @zoffixznet

Closing due to lack of interest.

Will address the mentioned security issue in a few days.

[p6rt]

@zoffixznet - Status changed from 'open' to 'rejected'