New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disallow strings as ops in cmp-ok in Test.pm6 #5347
Comments
From @zoffixznetThe 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'; 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" |
From @smlsSounds like throwing out the baby with the bath water, to me.
What makes you so sure? For example, the following works: my $name = "<"; my &op = CORE::.first(*.key eq "\&infix:<$name>" | "\&infix:«$name»").value; if &op { 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). |
The RT System itself - Status changed from 'new' to 'open' |
From @smlsOk, admittedly using `.first` to index the PseudoStash is a little silly. my &op = CORE::{"\&infix:<$name>"} // |
From @smlsOr this: my &op = (try &::("infix:<$name>")) // |
From @zoffixznetOn Sun May 29 08:51:45 2016, smls75@gmail.com wrote:
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 |
From @smls
Sorry, wasn't meant as an attack.
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>") |
From @zoffixznet
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 |
From @zoffixznetWhile wordy, this appears to work and removes all the issues raised in this ticket: sub cmp ($name, $a, $b) { op($a, $b); { I'll submit a PR and add extra tests for this to roast tonight or tomorrow. |
From @smlsUnfortunately, `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... |
From @zoffixznetClosing due to lack of interest. Will address the mentioned security issue in a few days. |
@zoffixznet - Status changed from 'open' to 'rejected' |
Migrated from rt.perl.org#128283 (status was 'rejected')
Searchable as RT128283$
The text was updated successfully, but these errors were encountered: