On 21 August 2011 00:56, Christian C. Salvadó <[email protected]> wrote:
>
> I agree with Peter, I'd expose only things I really want to. Personally, I
> always try to avoid `eval`, IMHO avoiding `eval` is not just about the risks
> of code evaluation, `eval` inhibits a lot of optimizations, for example, in
> your code, the variables `private` and `private2` are not statically
> referenced by anything, if you don't use `eval`, an implementation may
> optimize the function, to avoid storing the non-used values (it might not
> even create the variables if not referenced -no one will notice-). Using
> `eval` forces the implementation to keep access to all identifiers in the
> scope chain of the function, whether they are statically referenced or not.
> The Arguments object creation is another example where you can notice that
> `eval` impacts on function optimization, in most implementations the
> arguments object is not automatically created if you don't explicitly
> reference it on the function body. Using `eval` inside a function, forces
> its creation.
> Related:
> - SpiderMonkey Function Internals:
> https://developer.mozilla.org/En/SpiderMonkey/Internals/Functions
> - Some perf-tests on the Arguments object creation:
> http://jsperf.com/arguments-0-vs-a
>
I'm aware that in general eval is a bad idea - it was actually reading
about how it's an anti-pattern in JavaScript Patterns that made me
decide to have a play with it :) I know it forces the interpreter to
hang onto vars, but mostly I'd expect all those vars to be hung onto
anyway - they'd be referenced directly or indirectly from public
methods of the returned object in a real system. I didn't know about
the arguments optimisation though, that's interesting to know.
>>
>> I think you may have misread the regex - it checks for anything that's
>> not \w - i.e. any non-identifier character (I should've used \W rather
>> than [^\w] in retrospect). So it wouldn't allow alert('foo') through
>> (I've included an example of an alert not making it through).
>>
>
> Yeah, your RegExp will catch that case, but your `clean` function still has
> some vulnerabilities, for example, if you pass a tampered object to your
> `send` method:
> obj.send({
> match:function () {},
> replace: function() { return 'alert("I am teh hax0rz "+private)'; }
> });
>
> It will pass your validation and it will evaluate the code, granting access
> to all the identifiers in the scope of your `send` function... I would force
> string conversion before validating.
Aha, sneaky! Good find.
Cheers
--
Nick Morgan
http://skilldrick.co.uk
@skilldrick
Save our in-boxes! http://emailcharter.org
--
To view archived discussions from the original JSMentors Mailman list:
http://www.mail-archive.com/[email protected]/
To search via a non-Google archive, visit here:
http://www.mail-archive.com/[email protected]/
To unsubscribe from this group, send email to
[email protected]
