Running code generated in realtime in JavaScript with eval()How to scan Javascript for malicious code?How should I mitigate the XSS vulnerabilities in KnockoutJS described at mustache-security?What are the security issues with “eval()” in JavaScript?What's the danger of having some random, out of my control, JavaScript code running on my pages?Injected JavaScript CodeShould untrusted strings be sanitized server-side if they're inserted into the document body through document.createTextNode?Is eval() in JavaScript considered self-XSS?javascript eval() clarification
What names do Cormyr's people use?
Ghidra: Prepend memory segment in assembly listing view
Papers on arXiv solving the same problem at the same time
Billiard balls collision
When one problem is added to the previous one
Add 2 new columns to existing dataframe using apply
Nothing like a good ol' game of ModTen
Can you cast bonus action and reaction spells while already casting a spell?
Semantic difference between regular and irregular 'backen'
Evaluated vs. unevaluated Association
about to retire but not retired yet, employed but not working any more
Is first Ubuntu user root?
Removal of て in Japanese novels
Higman's lemma and a manuscript of Erdős and Rado
Redacting URLs as an email-phishing preventative?
How can I reorder triggered abilities in Arena?
Ordering a list of integers
Where does learning new skills fit into Agile?
Can an ISO file damage—or infect—the machine it's being burned on?
Boot Windows from SAN
Why do banks “park” their money at the European Central Bank?
Why does Windows store Wi-Fi passwords in a reversible format?
Command "root" and "subcommands"
Very slow boot time and poor perfomance
Running code generated in realtime in JavaScript with eval()
How to scan Javascript for malicious code?How should I mitigate the XSS vulnerabilities in KnockoutJS described at mustache-security?What are the security issues with “eval()” in JavaScript?What's the danger of having some random, out of my control, JavaScript code running on my pages?Injected JavaScript CodeShould untrusted strings be sanitized server-side if they're inserted into the document body through document.createTextNode?Is eval() in JavaScript considered self-XSS?javascript eval() clarification
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;
Consider a front-end JavaScript application where menu items needed to be shown or hidden based on somewhat simple logic (roles that user has and some other logical state).
A simple language was introduced to define this logic in a concise human-readable way and every menu item was assigned a string condition, which looks like this: "isLoggedIn() AND NOT role(PARTNER)"
.
In order to actually check this condition, a simple compiler was implemented, which translates this language into a valid JavaScript code string and executes it using eval()
returning the boolean result in the end.
The compiler works by replacing some constructs, like AND
, OR
and NOT
with valid JavaScript equivalents like &&
, ||
and !
using simple regular expressions:
private compileExpression(expression: string) ');
// Replacing "NOT"
expression = expression.replace(/(s
This turns "isLoggedIn() AND NOT role(PARTNER)"
into "Π.isLoggedIn() && !Π.role('PARTNER)"
, which is then executed by eval()
:
public matchCondition = (condition: string): boolean =>
// Using greek letter "P" for predicate (for shortness and uniqueness)
// noinspection NonAsciiCharacters
const Π = this.predicates;
const expression = this.compileExpression(condition);
const result = eval(expression);
return result;
The Π
is an object with simple predicate functions, which return boolean values like isLoggedIn(): boolean
and role(roleName: string): boolean
.
The expressions, that are being translated and executed are stored statically in local object as strings and are not accessible from global context. Also, all expressions are written by the developers and are not coming from users of the application in any way.
Is it safe to use eval()
this way or should it be avoided at all costs (e.g. "eval is evil", "never use eval", etc)?
What are the possible attack vectors, that could be used to compromise such eval usage?
If expression strings will be loaded from the HTTPS server using XHR/Fetch, will it change the situation security-wise?
The reason to introduce such language and not defining the rules in code directly is that it required that these conditions could be defined in string values, e.g. in a JSON file. The other reason is that such language is easier to read at a glance.
web-browser javascript
|
show 12 more comments
Consider a front-end JavaScript application where menu items needed to be shown or hidden based on somewhat simple logic (roles that user has and some other logical state).
A simple language was introduced to define this logic in a concise human-readable way and every menu item was assigned a string condition, which looks like this: "isLoggedIn() AND NOT role(PARTNER)"
.
In order to actually check this condition, a simple compiler was implemented, which translates this language into a valid JavaScript code string and executes it using eval()
returning the boolean result in the end.
The compiler works by replacing some constructs, like AND
, OR
and NOT
with valid JavaScript equivalents like &&
, ||
and !
using simple regular expressions:
private compileExpression(expression: string) ');
// Replacing "NOT"
expression = expression.replace(/(s
This turns "isLoggedIn() AND NOT role(PARTNER)"
into "Π.isLoggedIn() && !Π.role('PARTNER)"
, which is then executed by eval()
:
public matchCondition = (condition: string): boolean =>
// Using greek letter "P" for predicate (for shortness and uniqueness)
// noinspection NonAsciiCharacters
const Π = this.predicates;
const expression = this.compileExpression(condition);
const result = eval(expression);
return result;
The Π
is an object with simple predicate functions, which return boolean values like isLoggedIn(): boolean
and role(roleName: string): boolean
.
The expressions, that are being translated and executed are stored statically in local object as strings and are not accessible from global context. Also, all expressions are written by the developers and are not coming from users of the application in any way.
Is it safe to use eval()
this way or should it be avoided at all costs (e.g. "eval is evil", "never use eval", etc)?
What are the possible attack vectors, that could be used to compromise such eval usage?
If expression strings will be loaded from the HTTPS server using XHR/Fetch, will it change the situation security-wise?
The reason to introduce such language and not defining the rules in code directly is that it required that these conditions could be defined in string values, e.g. in a JSON file. The other reason is that such language is easier to read at a glance.
web-browser javascript
24
It's not just that eval is potentially insecure (which it is), it's also that solutions relying oneval
are typically hard-to-follow, confusing, and impossible to debug when things break. There are nearly always solutions that are, in all ways, better.
– Conor Mancone
Aug 13 at 12:52
21
I mean, I don't know your exact use-case, but I really doubt you have to go so far as building an entire language to make this happen. I suspect you're over thinking it. I've built a rules engines that allow the user to fully configure detailed conditions that have to be met before taking arbitrary actions, and did so without requiring either eval or a meta-language. In fact my rules engine sounds much more complicated. Things like: "If this is the customer id, and the status is one of these, and it was created more than X days but less than Y days ago, then send this email and this text
– Conor Mancone
Aug 13 at 13:53
4
I suppose I can't exactly break down a new system for you in comments, but effectively my answer is "I think you're using the wrong approach and should come up with a different solution". Eval is rarely the answer.
– Conor Mancone
Aug 13 at 13:59
18
I do wonder why your developers are not writing javascript straight away. Your syntax doesn't seem to have that much of a conciseness and readability advantage, but the regular expression compiler is very limited.
– Bergi
Aug 13 at 20:56
9
If all you are doing is to hide and show menus, why not use CSS selectors instead of Javascript?
– Ángel
Aug 13 at 22:24
|
show 12 more comments
Consider a front-end JavaScript application where menu items needed to be shown or hidden based on somewhat simple logic (roles that user has and some other logical state).
A simple language was introduced to define this logic in a concise human-readable way and every menu item was assigned a string condition, which looks like this: "isLoggedIn() AND NOT role(PARTNER)"
.
In order to actually check this condition, a simple compiler was implemented, which translates this language into a valid JavaScript code string and executes it using eval()
returning the boolean result in the end.
The compiler works by replacing some constructs, like AND
, OR
and NOT
with valid JavaScript equivalents like &&
, ||
and !
using simple regular expressions:
private compileExpression(expression: string) ');
// Replacing "NOT"
expression = expression.replace(/(s
This turns "isLoggedIn() AND NOT role(PARTNER)"
into "Π.isLoggedIn() && !Π.role('PARTNER)"
, which is then executed by eval()
:
public matchCondition = (condition: string): boolean =>
// Using greek letter "P" for predicate (for shortness and uniqueness)
// noinspection NonAsciiCharacters
const Π = this.predicates;
const expression = this.compileExpression(condition);
const result = eval(expression);
return result;
The Π
is an object with simple predicate functions, which return boolean values like isLoggedIn(): boolean
and role(roleName: string): boolean
.
The expressions, that are being translated and executed are stored statically in local object as strings and are not accessible from global context. Also, all expressions are written by the developers and are not coming from users of the application in any way.
Is it safe to use eval()
this way or should it be avoided at all costs (e.g. "eval is evil", "never use eval", etc)?
What are the possible attack vectors, that could be used to compromise such eval usage?
If expression strings will be loaded from the HTTPS server using XHR/Fetch, will it change the situation security-wise?
The reason to introduce such language and not defining the rules in code directly is that it required that these conditions could be defined in string values, e.g. in a JSON file. The other reason is that such language is easier to read at a glance.
web-browser javascript
Consider a front-end JavaScript application where menu items needed to be shown or hidden based on somewhat simple logic (roles that user has and some other logical state).
A simple language was introduced to define this logic in a concise human-readable way and every menu item was assigned a string condition, which looks like this: "isLoggedIn() AND NOT role(PARTNER)"
.
In order to actually check this condition, a simple compiler was implemented, which translates this language into a valid JavaScript code string and executes it using eval()
returning the boolean result in the end.
The compiler works by replacing some constructs, like AND
, OR
and NOT
with valid JavaScript equivalents like &&
, ||
and !
using simple regular expressions:
private compileExpression(expression: string) ');
// Replacing "NOT"
expression = expression.replace(/(s
This turns "isLoggedIn() AND NOT role(PARTNER)"
into "Π.isLoggedIn() && !Π.role('PARTNER)"
, which is then executed by eval()
:
public matchCondition = (condition: string): boolean =>
// Using greek letter "P" for predicate (for shortness and uniqueness)
// noinspection NonAsciiCharacters
const Π = this.predicates;
const expression = this.compileExpression(condition);
const result = eval(expression);
return result;
The Π
is an object with simple predicate functions, which return boolean values like isLoggedIn(): boolean
and role(roleName: string): boolean
.
The expressions, that are being translated and executed are stored statically in local object as strings and are not accessible from global context. Also, all expressions are written by the developers and are not coming from users of the application in any way.
Is it safe to use eval()
this way or should it be avoided at all costs (e.g. "eval is evil", "never use eval", etc)?
What are the possible attack vectors, that could be used to compromise such eval usage?
If expression strings will be loaded from the HTTPS server using XHR/Fetch, will it change the situation security-wise?
The reason to introduce such language and not defining the rules in code directly is that it required that these conditions could be defined in string values, e.g. in a JSON file. The other reason is that such language is easier to read at a glance.
web-browser javascript
web-browser javascript
edited Aug 18 at 18:48
Slava Fomin II
asked Aug 13 at 11:36
Slava Fomin IISlava Fomin II
1621 silver badge7 bronze badges
1621 silver badge7 bronze badges
24
It's not just that eval is potentially insecure (which it is), it's also that solutions relying oneval
are typically hard-to-follow, confusing, and impossible to debug when things break. There are nearly always solutions that are, in all ways, better.
– Conor Mancone
Aug 13 at 12:52
21
I mean, I don't know your exact use-case, but I really doubt you have to go so far as building an entire language to make this happen. I suspect you're over thinking it. I've built a rules engines that allow the user to fully configure detailed conditions that have to be met before taking arbitrary actions, and did so without requiring either eval or a meta-language. In fact my rules engine sounds much more complicated. Things like: "If this is the customer id, and the status is one of these, and it was created more than X days but less than Y days ago, then send this email and this text
– Conor Mancone
Aug 13 at 13:53
4
I suppose I can't exactly break down a new system for you in comments, but effectively my answer is "I think you're using the wrong approach and should come up with a different solution". Eval is rarely the answer.
– Conor Mancone
Aug 13 at 13:59
18
I do wonder why your developers are not writing javascript straight away. Your syntax doesn't seem to have that much of a conciseness and readability advantage, but the regular expression compiler is very limited.
– Bergi
Aug 13 at 20:56
9
If all you are doing is to hide and show menus, why not use CSS selectors instead of Javascript?
– Ángel
Aug 13 at 22:24
|
show 12 more comments
24
It's not just that eval is potentially insecure (which it is), it's also that solutions relying oneval
are typically hard-to-follow, confusing, and impossible to debug when things break. There are nearly always solutions that are, in all ways, better.
– Conor Mancone
Aug 13 at 12:52
21
I mean, I don't know your exact use-case, but I really doubt you have to go so far as building an entire language to make this happen. I suspect you're over thinking it. I've built a rules engines that allow the user to fully configure detailed conditions that have to be met before taking arbitrary actions, and did so without requiring either eval or a meta-language. In fact my rules engine sounds much more complicated. Things like: "If this is the customer id, and the status is one of these, and it was created more than X days but less than Y days ago, then send this email and this text
– Conor Mancone
Aug 13 at 13:53
4
I suppose I can't exactly break down a new system for you in comments, but effectively my answer is "I think you're using the wrong approach and should come up with a different solution". Eval is rarely the answer.
– Conor Mancone
Aug 13 at 13:59
18
I do wonder why your developers are not writing javascript straight away. Your syntax doesn't seem to have that much of a conciseness and readability advantage, but the regular expression compiler is very limited.
– Bergi
Aug 13 at 20:56
9
If all you are doing is to hide and show menus, why not use CSS selectors instead of Javascript?
– Ángel
Aug 13 at 22:24
24
24
It's not just that eval is potentially insecure (which it is), it's also that solutions relying on
eval
are typically hard-to-follow, confusing, and impossible to debug when things break. There are nearly always solutions that are, in all ways, better.– Conor Mancone
Aug 13 at 12:52
It's not just that eval is potentially insecure (which it is), it's also that solutions relying on
eval
are typically hard-to-follow, confusing, and impossible to debug when things break. There are nearly always solutions that are, in all ways, better.– Conor Mancone
Aug 13 at 12:52
21
21
I mean, I don't know your exact use-case, but I really doubt you have to go so far as building an entire language to make this happen. I suspect you're over thinking it. I've built a rules engines that allow the user to fully configure detailed conditions that have to be met before taking arbitrary actions, and did so without requiring either eval or a meta-language. In fact my rules engine sounds much more complicated. Things like: "If this is the customer id, and the status is one of these, and it was created more than X days but less than Y days ago, then send this email and this text
– Conor Mancone
Aug 13 at 13:53
I mean, I don't know your exact use-case, but I really doubt you have to go so far as building an entire language to make this happen. I suspect you're over thinking it. I've built a rules engines that allow the user to fully configure detailed conditions that have to be met before taking arbitrary actions, and did so without requiring either eval or a meta-language. In fact my rules engine sounds much more complicated. Things like: "If this is the customer id, and the status is one of these, and it was created more than X days but less than Y days ago, then send this email and this text
– Conor Mancone
Aug 13 at 13:53
4
4
I suppose I can't exactly break down a new system for you in comments, but effectively my answer is "I think you're using the wrong approach and should come up with a different solution". Eval is rarely the answer.
– Conor Mancone
Aug 13 at 13:59
I suppose I can't exactly break down a new system for you in comments, but effectively my answer is "I think you're using the wrong approach and should come up with a different solution". Eval is rarely the answer.
– Conor Mancone
Aug 13 at 13:59
18
18
I do wonder why your developers are not writing javascript straight away. Your syntax doesn't seem to have that much of a conciseness and readability advantage, but the regular expression compiler is very limited.
– Bergi
Aug 13 at 20:56
I do wonder why your developers are not writing javascript straight away. Your syntax doesn't seem to have that much of a conciseness and readability advantage, but the regular expression compiler is very limited.
– Bergi
Aug 13 at 20:56
9
9
If all you are doing is to hide and show menus, why not use CSS selectors instead of Javascript?
– Ángel
Aug 13 at 22:24
If all you are doing is to hide and show menus, why not use CSS selectors instead of Javascript?
– Ángel
Aug 13 at 22:24
|
show 12 more comments
5 Answers
5
active
oldest
votes
Using eval
in this context doesn't create any vulnerability, as long as an attacker can't interfere with the arguments passed to matchCondition
.
If you find it easier to read / program it this way, and you're confident that no untrusted input will ever go into your expression compiler, then go for it.
eval
isn't evil, untrusted data is.
Please note that it's entirely possible to avoid eval
, by extracting the predicates then handling them with your custom functions, for example:
if (predicate === 'isLoggedIn()')
return Π.isLoggedIn();
The code in my question is not even trying to somehow sanitize the passed expression, I guess this is obvious. I believe the main question is how would attacker putalert`1`
(or any other malicious code) into the expression in the first place?
– Slava Fomin II
Aug 13 at 13:13
Oh, I missed the "all expressions are written by the developers" part. Edited my answer.
– Benoit Esnard
Aug 13 at 13:25
3
Another alternative for replacing itreturn Π[predicate]();
where the passed in predicate is the string"isLoggedIn"
. You can validate it, of course, to avoid a TypeError when running the code or to avoid any potential other situations but there is very rarely a good reason to useeval
especially if all you need is to call some function dynamically. In JS you can also pass functions as parameters, so it's entirely possible to do that instead of a string and execute it at the end.
– VLAZ
Aug 14 at 6:45
2
Although I realize this is infosec.SE and what the OP is doing isn't necessarily insecure,eval
is still evil for plenty of other reasons.
– Jared Smith
Aug 15 at 12:35
add a comment |
Today, everything is written by developers. Next month or next year, someone will say "hey, why not let the users write those themselves?" Bam.
Also, even if the rules are written by the developers only, do they or will they include any user-originated data? Something like titles, names, categories, for instance? This could quickly lead to an XSS attack.
Your regular expressions are so "open" (using lots of .*
without any validation) that if anything untoward gets in, it will slip through directly to the eval
in a minute.
At the very least, if you want to keep eval
, you should have a lot stricter expressions instead of .*
. But those can quickly become either difficult to understand, or a hindrance for many practical cases.
1
The point regarding the future proofing makes sense, thank you.
– Slava Fomin II
Aug 14 at 1:08
1
It doesn't matter whether the rules are written by the users, or by professional devs, all that matters is whether you offer unsanitised data/code to other users. In the majority of cases,eval
is fine, because a malicious actor can only affect code that he is running himself.
– Mike Brockington
Aug 14 at 16:04
I think you give developers too much credit here! :)
– gbjbaanb
Aug 15 at 13:32
add a comment |
Appearances and expectations
If something looks like a safe expression, people will probably treat it like one. If a field looks like any other data-field, people (even developers) will probably put untrusted data in there. If something is evaluated with full level application access, it should look and feel like code.
Another problem are subtle bugs in your pre-compiler, which could introduce unwanted bugs/security flaws. Most vulnerabilities start with unwanted bugs, before a malicious attacker can exploit something. And a new meta-language without proper vetting/tests and strongly defined syntax is just another layer of confusion and bugs waiting to happen.
If only developers write code for your conditions, why not just use plain JavaScript? The meta-language brings hardly any benefit. And code is handled by people like code.
add a comment |
Front-end javascript itself is completely at the will of the client running the code. If you are depending on front-end javascript for security, you've already failed to secure your application. Forget eval. The client can replace your entire website with their own implementation if they desire. Thus, your server should validate everything that the client asks it to do.
In your case, you should ask yourself if it's a security violation for users to see menu items that they're not privileged enough to see. If so, then the server should not deliver those menu items to the client, because users can look at any javascript you deliver to them. If not, then you don't have any security concerns at all -- as others have mentioned, eval is only "bad" if it is run against unsanitized code. Since your eval'd code is currently sanitized, then I don't believe you have security concerns.
add a comment |
As others have said, as long as your rules really are coming only from trusted developers, there shouldn't be any security holes from using eval
.
However, eval
has plenty of other disadvantages, in terms of complexity, maintainability, debuggability, etc. And using regular expressions plus eval
could easily result in problems down the road, depending on how your application grows.
I also think it's possible that you're overestimating the difficulty of writing your own interpreter; open-source libraries are mature enough that you can create a pretty nice interpreter even if (like me) you're not well-versed in parser and compiler implementation. Here's a grammar for PEG.js that I put together in about 15 minutes based on your problem description. For demonstration purposes, it just returns the predicate names; to use it, you'd change it to return a function that takes the predicates object and invokes the appropriate predicate, but hopefully this is enough to give you an idea of one possible approach.
OrExpression
= head:AndExpression tail:(_ ("OR") _ AndExpression)*
return tail.reduce(function(result, element) element[3];
, head);
AndExpression
= head:NotExpression tail:(_ ("AND") _ NotExpression)*
return tail.reduce(function(result, element)
return result && element[3];
, head);
NotExpression
= "NOT" _ expr:NotExpression return !expr;
/ "(" _ expr:OrExpression _ ")" return expr;
/ Predicate;
Predicate
= _ predicate:[A-Z]+ _ "(" _ arg:[A-Z]* _ ")"
return predicate.join('') + '(' + arg.join('') + ')';
_ "whitespace"
= [ tnr]*
add a comment |
Your Answer
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "162"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
noCode: true, onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fsecurity.stackexchange.com%2fquestions%2f215219%2frunning-code-generated-in-realtime-in-javascript-with-eval%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
5 Answers
5
active
oldest
votes
5 Answers
5
active
oldest
votes
active
oldest
votes
active
oldest
votes
Using eval
in this context doesn't create any vulnerability, as long as an attacker can't interfere with the arguments passed to matchCondition
.
If you find it easier to read / program it this way, and you're confident that no untrusted input will ever go into your expression compiler, then go for it.
eval
isn't evil, untrusted data is.
Please note that it's entirely possible to avoid eval
, by extracting the predicates then handling them with your custom functions, for example:
if (predicate === 'isLoggedIn()')
return Π.isLoggedIn();
The code in my question is not even trying to somehow sanitize the passed expression, I guess this is obvious. I believe the main question is how would attacker putalert`1`
(or any other malicious code) into the expression in the first place?
– Slava Fomin II
Aug 13 at 13:13
Oh, I missed the "all expressions are written by the developers" part. Edited my answer.
– Benoit Esnard
Aug 13 at 13:25
3
Another alternative for replacing itreturn Π[predicate]();
where the passed in predicate is the string"isLoggedIn"
. You can validate it, of course, to avoid a TypeError when running the code or to avoid any potential other situations but there is very rarely a good reason to useeval
especially if all you need is to call some function dynamically. In JS you can also pass functions as parameters, so it's entirely possible to do that instead of a string and execute it at the end.
– VLAZ
Aug 14 at 6:45
2
Although I realize this is infosec.SE and what the OP is doing isn't necessarily insecure,eval
is still evil for plenty of other reasons.
– Jared Smith
Aug 15 at 12:35
add a comment |
Using eval
in this context doesn't create any vulnerability, as long as an attacker can't interfere with the arguments passed to matchCondition
.
If you find it easier to read / program it this way, and you're confident that no untrusted input will ever go into your expression compiler, then go for it.
eval
isn't evil, untrusted data is.
Please note that it's entirely possible to avoid eval
, by extracting the predicates then handling them with your custom functions, for example:
if (predicate === 'isLoggedIn()')
return Π.isLoggedIn();
The code in my question is not even trying to somehow sanitize the passed expression, I guess this is obvious. I believe the main question is how would attacker putalert`1`
(or any other malicious code) into the expression in the first place?
– Slava Fomin II
Aug 13 at 13:13
Oh, I missed the "all expressions are written by the developers" part. Edited my answer.
– Benoit Esnard
Aug 13 at 13:25
3
Another alternative for replacing itreturn Π[predicate]();
where the passed in predicate is the string"isLoggedIn"
. You can validate it, of course, to avoid a TypeError when running the code or to avoid any potential other situations but there is very rarely a good reason to useeval
especially if all you need is to call some function dynamically. In JS you can also pass functions as parameters, so it's entirely possible to do that instead of a string and execute it at the end.
– VLAZ
Aug 14 at 6:45
2
Although I realize this is infosec.SE and what the OP is doing isn't necessarily insecure,eval
is still evil for plenty of other reasons.
– Jared Smith
Aug 15 at 12:35
add a comment |
Using eval
in this context doesn't create any vulnerability, as long as an attacker can't interfere with the arguments passed to matchCondition
.
If you find it easier to read / program it this way, and you're confident that no untrusted input will ever go into your expression compiler, then go for it.
eval
isn't evil, untrusted data is.
Please note that it's entirely possible to avoid eval
, by extracting the predicates then handling them with your custom functions, for example:
if (predicate === 'isLoggedIn()')
return Π.isLoggedIn();
Using eval
in this context doesn't create any vulnerability, as long as an attacker can't interfere with the arguments passed to matchCondition
.
If you find it easier to read / program it this way, and you're confident that no untrusted input will ever go into your expression compiler, then go for it.
eval
isn't evil, untrusted data is.
Please note that it's entirely possible to avoid eval
, by extracting the predicates then handling them with your custom functions, for example:
if (predicate === 'isLoggedIn()')
return Π.isLoggedIn();
edited Aug 14 at 12:03
answered Aug 13 at 12:26
Benoit EsnardBenoit Esnard
11.2k7 gold badges56 silver badges58 bronze badges
11.2k7 gold badges56 silver badges58 bronze badges
The code in my question is not even trying to somehow sanitize the passed expression, I guess this is obvious. I believe the main question is how would attacker putalert`1`
(or any other malicious code) into the expression in the first place?
– Slava Fomin II
Aug 13 at 13:13
Oh, I missed the "all expressions are written by the developers" part. Edited my answer.
– Benoit Esnard
Aug 13 at 13:25
3
Another alternative for replacing itreturn Π[predicate]();
where the passed in predicate is the string"isLoggedIn"
. You can validate it, of course, to avoid a TypeError when running the code or to avoid any potential other situations but there is very rarely a good reason to useeval
especially if all you need is to call some function dynamically. In JS you can also pass functions as parameters, so it's entirely possible to do that instead of a string and execute it at the end.
– VLAZ
Aug 14 at 6:45
2
Although I realize this is infosec.SE and what the OP is doing isn't necessarily insecure,eval
is still evil for plenty of other reasons.
– Jared Smith
Aug 15 at 12:35
add a comment |
The code in my question is not even trying to somehow sanitize the passed expression, I guess this is obvious. I believe the main question is how would attacker putalert`1`
(or any other malicious code) into the expression in the first place?
– Slava Fomin II
Aug 13 at 13:13
Oh, I missed the "all expressions are written by the developers" part. Edited my answer.
– Benoit Esnard
Aug 13 at 13:25
3
Another alternative for replacing itreturn Π[predicate]();
where the passed in predicate is the string"isLoggedIn"
. You can validate it, of course, to avoid a TypeError when running the code or to avoid any potential other situations but there is very rarely a good reason to useeval
especially if all you need is to call some function dynamically. In JS you can also pass functions as parameters, so it's entirely possible to do that instead of a string and execute it at the end.
– VLAZ
Aug 14 at 6:45
2
Although I realize this is infosec.SE and what the OP is doing isn't necessarily insecure,eval
is still evil for plenty of other reasons.
– Jared Smith
Aug 15 at 12:35
The code in my question is not even trying to somehow sanitize the passed expression, I guess this is obvious. I believe the main question is how would attacker put
alert`1`
(or any other malicious code) into the expression in the first place?– Slava Fomin II
Aug 13 at 13:13
The code in my question is not even trying to somehow sanitize the passed expression, I guess this is obvious. I believe the main question is how would attacker put
alert`1`
(or any other malicious code) into the expression in the first place?– Slava Fomin II
Aug 13 at 13:13
Oh, I missed the "all expressions are written by the developers" part. Edited my answer.
– Benoit Esnard
Aug 13 at 13:25
Oh, I missed the "all expressions are written by the developers" part. Edited my answer.
– Benoit Esnard
Aug 13 at 13:25
3
3
Another alternative for replacing it
return Π[predicate]();
where the passed in predicate is the string "isLoggedIn"
. You can validate it, of course, to avoid a TypeError when running the code or to avoid any potential other situations but there is very rarely a good reason to use eval
especially if all you need is to call some function dynamically. In JS you can also pass functions as parameters, so it's entirely possible to do that instead of a string and execute it at the end.– VLAZ
Aug 14 at 6:45
Another alternative for replacing it
return Π[predicate]();
where the passed in predicate is the string "isLoggedIn"
. You can validate it, of course, to avoid a TypeError when running the code or to avoid any potential other situations but there is very rarely a good reason to use eval
especially if all you need is to call some function dynamically. In JS you can also pass functions as parameters, so it's entirely possible to do that instead of a string and execute it at the end.– VLAZ
Aug 14 at 6:45
2
2
Although I realize this is infosec.SE and what the OP is doing isn't necessarily insecure,
eval
is still evil for plenty of other reasons.– Jared Smith
Aug 15 at 12:35
Although I realize this is infosec.SE and what the OP is doing isn't necessarily insecure,
eval
is still evil for plenty of other reasons.– Jared Smith
Aug 15 at 12:35
add a comment |
Today, everything is written by developers. Next month or next year, someone will say "hey, why not let the users write those themselves?" Bam.
Also, even if the rules are written by the developers only, do they or will they include any user-originated data? Something like titles, names, categories, for instance? This could quickly lead to an XSS attack.
Your regular expressions are so "open" (using lots of .*
without any validation) that if anything untoward gets in, it will slip through directly to the eval
in a minute.
At the very least, if you want to keep eval
, you should have a lot stricter expressions instead of .*
. But those can quickly become either difficult to understand, or a hindrance for many practical cases.
1
The point regarding the future proofing makes sense, thank you.
– Slava Fomin II
Aug 14 at 1:08
1
It doesn't matter whether the rules are written by the users, or by professional devs, all that matters is whether you offer unsanitised data/code to other users. In the majority of cases,eval
is fine, because a malicious actor can only affect code that he is running himself.
– Mike Brockington
Aug 14 at 16:04
I think you give developers too much credit here! :)
– gbjbaanb
Aug 15 at 13:32
add a comment |
Today, everything is written by developers. Next month or next year, someone will say "hey, why not let the users write those themselves?" Bam.
Also, even if the rules are written by the developers only, do they or will they include any user-originated data? Something like titles, names, categories, for instance? This could quickly lead to an XSS attack.
Your regular expressions are so "open" (using lots of .*
without any validation) that if anything untoward gets in, it will slip through directly to the eval
in a minute.
At the very least, if you want to keep eval
, you should have a lot stricter expressions instead of .*
. But those can quickly become either difficult to understand, or a hindrance for many practical cases.
1
The point regarding the future proofing makes sense, thank you.
– Slava Fomin II
Aug 14 at 1:08
1
It doesn't matter whether the rules are written by the users, or by professional devs, all that matters is whether you offer unsanitised data/code to other users. In the majority of cases,eval
is fine, because a malicious actor can only affect code that he is running himself.
– Mike Brockington
Aug 14 at 16:04
I think you give developers too much credit here! :)
– gbjbaanb
Aug 15 at 13:32
add a comment |
Today, everything is written by developers. Next month or next year, someone will say "hey, why not let the users write those themselves?" Bam.
Also, even if the rules are written by the developers only, do they or will they include any user-originated data? Something like titles, names, categories, for instance? This could quickly lead to an XSS attack.
Your regular expressions are so "open" (using lots of .*
without any validation) that if anything untoward gets in, it will slip through directly to the eval
in a minute.
At the very least, if you want to keep eval
, you should have a lot stricter expressions instead of .*
. But those can quickly become either difficult to understand, or a hindrance for many practical cases.
Today, everything is written by developers. Next month or next year, someone will say "hey, why not let the users write those themselves?" Bam.
Also, even if the rules are written by the developers only, do they or will they include any user-originated data? Something like titles, names, categories, for instance? This could quickly lead to an XSS attack.
Your regular expressions are so "open" (using lots of .*
without any validation) that if anything untoward gets in, it will slip through directly to the eval
in a minute.
At the very least, if you want to keep eval
, you should have a lot stricter expressions instead of .*
. But those can quickly become either difficult to understand, or a hindrance for many practical cases.
answered Aug 13 at 22:26
jcaronjcaron
1,3801 gold badge8 silver badges17 bronze badges
1,3801 gold badge8 silver badges17 bronze badges
1
The point regarding the future proofing makes sense, thank you.
– Slava Fomin II
Aug 14 at 1:08
1
It doesn't matter whether the rules are written by the users, or by professional devs, all that matters is whether you offer unsanitised data/code to other users. In the majority of cases,eval
is fine, because a malicious actor can only affect code that he is running himself.
– Mike Brockington
Aug 14 at 16:04
I think you give developers too much credit here! :)
– gbjbaanb
Aug 15 at 13:32
add a comment |
1
The point regarding the future proofing makes sense, thank you.
– Slava Fomin II
Aug 14 at 1:08
1
It doesn't matter whether the rules are written by the users, or by professional devs, all that matters is whether you offer unsanitised data/code to other users. In the majority of cases,eval
is fine, because a malicious actor can only affect code that he is running himself.
– Mike Brockington
Aug 14 at 16:04
I think you give developers too much credit here! :)
– gbjbaanb
Aug 15 at 13:32
1
1
The point regarding the future proofing makes sense, thank you.
– Slava Fomin II
Aug 14 at 1:08
The point regarding the future proofing makes sense, thank you.
– Slava Fomin II
Aug 14 at 1:08
1
1
It doesn't matter whether the rules are written by the users, or by professional devs, all that matters is whether you offer unsanitised data/code to other users. In the majority of cases,
eval
is fine, because a malicious actor can only affect code that he is running himself.– Mike Brockington
Aug 14 at 16:04
It doesn't matter whether the rules are written by the users, or by professional devs, all that matters is whether you offer unsanitised data/code to other users. In the majority of cases,
eval
is fine, because a malicious actor can only affect code that he is running himself.– Mike Brockington
Aug 14 at 16:04
I think you give developers too much credit here! :)
– gbjbaanb
Aug 15 at 13:32
I think you give developers too much credit here! :)
– gbjbaanb
Aug 15 at 13:32
add a comment |
Appearances and expectations
If something looks like a safe expression, people will probably treat it like one. If a field looks like any other data-field, people (even developers) will probably put untrusted data in there. If something is evaluated with full level application access, it should look and feel like code.
Another problem are subtle bugs in your pre-compiler, which could introduce unwanted bugs/security flaws. Most vulnerabilities start with unwanted bugs, before a malicious attacker can exploit something. And a new meta-language without proper vetting/tests and strongly defined syntax is just another layer of confusion and bugs waiting to happen.
If only developers write code for your conditions, why not just use plain JavaScript? The meta-language brings hardly any benefit. And code is handled by people like code.
add a comment |
Appearances and expectations
If something looks like a safe expression, people will probably treat it like one. If a field looks like any other data-field, people (even developers) will probably put untrusted data in there. If something is evaluated with full level application access, it should look and feel like code.
Another problem are subtle bugs in your pre-compiler, which could introduce unwanted bugs/security flaws. Most vulnerabilities start with unwanted bugs, before a malicious attacker can exploit something. And a new meta-language without proper vetting/tests and strongly defined syntax is just another layer of confusion and bugs waiting to happen.
If only developers write code for your conditions, why not just use plain JavaScript? The meta-language brings hardly any benefit. And code is handled by people like code.
add a comment |
Appearances and expectations
If something looks like a safe expression, people will probably treat it like one. If a field looks like any other data-field, people (even developers) will probably put untrusted data in there. If something is evaluated with full level application access, it should look and feel like code.
Another problem are subtle bugs in your pre-compiler, which could introduce unwanted bugs/security flaws. Most vulnerabilities start with unwanted bugs, before a malicious attacker can exploit something. And a new meta-language without proper vetting/tests and strongly defined syntax is just another layer of confusion and bugs waiting to happen.
If only developers write code for your conditions, why not just use plain JavaScript? The meta-language brings hardly any benefit. And code is handled by people like code.
Appearances and expectations
If something looks like a safe expression, people will probably treat it like one. If a field looks like any other data-field, people (even developers) will probably put untrusted data in there. If something is evaluated with full level application access, it should look and feel like code.
Another problem are subtle bugs in your pre-compiler, which could introduce unwanted bugs/security flaws. Most vulnerabilities start with unwanted bugs, before a malicious attacker can exploit something. And a new meta-language without proper vetting/tests and strongly defined syntax is just another layer of confusion and bugs waiting to happen.
If only developers write code for your conditions, why not just use plain JavaScript? The meta-language brings hardly any benefit. And code is handled by people like code.
answered Aug 14 at 11:10
FalcoFalco
1,3528 silver badges14 bronze badges
1,3528 silver badges14 bronze badges
add a comment |
add a comment |
Front-end javascript itself is completely at the will of the client running the code. If you are depending on front-end javascript for security, you've already failed to secure your application. Forget eval. The client can replace your entire website with their own implementation if they desire. Thus, your server should validate everything that the client asks it to do.
In your case, you should ask yourself if it's a security violation for users to see menu items that they're not privileged enough to see. If so, then the server should not deliver those menu items to the client, because users can look at any javascript you deliver to them. If not, then you don't have any security concerns at all -- as others have mentioned, eval is only "bad" if it is run against unsanitized code. Since your eval'd code is currently sanitized, then I don't believe you have security concerns.
add a comment |
Front-end javascript itself is completely at the will of the client running the code. If you are depending on front-end javascript for security, you've already failed to secure your application. Forget eval. The client can replace your entire website with their own implementation if they desire. Thus, your server should validate everything that the client asks it to do.
In your case, you should ask yourself if it's a security violation for users to see menu items that they're not privileged enough to see. If so, then the server should not deliver those menu items to the client, because users can look at any javascript you deliver to them. If not, then you don't have any security concerns at all -- as others have mentioned, eval is only "bad" if it is run against unsanitized code. Since your eval'd code is currently sanitized, then I don't believe you have security concerns.
add a comment |
Front-end javascript itself is completely at the will of the client running the code. If you are depending on front-end javascript for security, you've already failed to secure your application. Forget eval. The client can replace your entire website with their own implementation if they desire. Thus, your server should validate everything that the client asks it to do.
In your case, you should ask yourself if it's a security violation for users to see menu items that they're not privileged enough to see. If so, then the server should not deliver those menu items to the client, because users can look at any javascript you deliver to them. If not, then you don't have any security concerns at all -- as others have mentioned, eval is only "bad" if it is run against unsanitized code. Since your eval'd code is currently sanitized, then I don't believe you have security concerns.
Front-end javascript itself is completely at the will of the client running the code. If you are depending on front-end javascript for security, you've already failed to secure your application. Forget eval. The client can replace your entire website with their own implementation if they desire. Thus, your server should validate everything that the client asks it to do.
In your case, you should ask yourself if it's a security violation for users to see menu items that they're not privileged enough to see. If so, then the server should not deliver those menu items to the client, because users can look at any javascript you deliver to them. If not, then you don't have any security concerns at all -- as others have mentioned, eval is only "bad" if it is run against unsanitized code. Since your eval'd code is currently sanitized, then I don't believe you have security concerns.
edited Aug 15 at 19:30
answered Aug 15 at 19:04
bvoyelrbvoyelr
1352 bronze badges
1352 bronze badges
add a comment |
add a comment |
As others have said, as long as your rules really are coming only from trusted developers, there shouldn't be any security holes from using eval
.
However, eval
has plenty of other disadvantages, in terms of complexity, maintainability, debuggability, etc. And using regular expressions plus eval
could easily result in problems down the road, depending on how your application grows.
I also think it's possible that you're overestimating the difficulty of writing your own interpreter; open-source libraries are mature enough that you can create a pretty nice interpreter even if (like me) you're not well-versed in parser and compiler implementation. Here's a grammar for PEG.js that I put together in about 15 minutes based on your problem description. For demonstration purposes, it just returns the predicate names; to use it, you'd change it to return a function that takes the predicates object and invokes the appropriate predicate, but hopefully this is enough to give you an idea of one possible approach.
OrExpression
= head:AndExpression tail:(_ ("OR") _ AndExpression)*
return tail.reduce(function(result, element) element[3];
, head);
AndExpression
= head:NotExpression tail:(_ ("AND") _ NotExpression)*
return tail.reduce(function(result, element)
return result && element[3];
, head);
NotExpression
= "NOT" _ expr:NotExpression return !expr;
/ "(" _ expr:OrExpression _ ")" return expr;
/ Predicate;
Predicate
= _ predicate:[A-Z]+ _ "(" _ arg:[A-Z]* _ ")"
return predicate.join('') + '(' + arg.join('') + ')';
_ "whitespace"
= [ tnr]*
add a comment |
As others have said, as long as your rules really are coming only from trusted developers, there shouldn't be any security holes from using eval
.
However, eval
has plenty of other disadvantages, in terms of complexity, maintainability, debuggability, etc. And using regular expressions plus eval
could easily result in problems down the road, depending on how your application grows.
I also think it's possible that you're overestimating the difficulty of writing your own interpreter; open-source libraries are mature enough that you can create a pretty nice interpreter even if (like me) you're not well-versed in parser and compiler implementation. Here's a grammar for PEG.js that I put together in about 15 minutes based on your problem description. For demonstration purposes, it just returns the predicate names; to use it, you'd change it to return a function that takes the predicates object and invokes the appropriate predicate, but hopefully this is enough to give you an idea of one possible approach.
OrExpression
= head:AndExpression tail:(_ ("OR") _ AndExpression)*
return tail.reduce(function(result, element) element[3];
, head);
AndExpression
= head:NotExpression tail:(_ ("AND") _ NotExpression)*
return tail.reduce(function(result, element)
return result && element[3];
, head);
NotExpression
= "NOT" _ expr:NotExpression return !expr;
/ "(" _ expr:OrExpression _ ")" return expr;
/ Predicate;
Predicate
= _ predicate:[A-Z]+ _ "(" _ arg:[A-Z]* _ ")"
return predicate.join('') + '(' + arg.join('') + ')';
_ "whitespace"
= [ tnr]*
add a comment |
As others have said, as long as your rules really are coming only from trusted developers, there shouldn't be any security holes from using eval
.
However, eval
has plenty of other disadvantages, in terms of complexity, maintainability, debuggability, etc. And using regular expressions plus eval
could easily result in problems down the road, depending on how your application grows.
I also think it's possible that you're overestimating the difficulty of writing your own interpreter; open-source libraries are mature enough that you can create a pretty nice interpreter even if (like me) you're not well-versed in parser and compiler implementation. Here's a grammar for PEG.js that I put together in about 15 minutes based on your problem description. For demonstration purposes, it just returns the predicate names; to use it, you'd change it to return a function that takes the predicates object and invokes the appropriate predicate, but hopefully this is enough to give you an idea of one possible approach.
OrExpression
= head:AndExpression tail:(_ ("OR") _ AndExpression)*
return tail.reduce(function(result, element) element[3];
, head);
AndExpression
= head:NotExpression tail:(_ ("AND") _ NotExpression)*
return tail.reduce(function(result, element)
return result && element[3];
, head);
NotExpression
= "NOT" _ expr:NotExpression return !expr;
/ "(" _ expr:OrExpression _ ")" return expr;
/ Predicate;
Predicate
= _ predicate:[A-Z]+ _ "(" _ arg:[A-Z]* _ ")"
return predicate.join('') + '(' + arg.join('') + ')';
_ "whitespace"
= [ tnr]*
As others have said, as long as your rules really are coming only from trusted developers, there shouldn't be any security holes from using eval
.
However, eval
has plenty of other disadvantages, in terms of complexity, maintainability, debuggability, etc. And using regular expressions plus eval
could easily result in problems down the road, depending on how your application grows.
I also think it's possible that you're overestimating the difficulty of writing your own interpreter; open-source libraries are mature enough that you can create a pretty nice interpreter even if (like me) you're not well-versed in parser and compiler implementation. Here's a grammar for PEG.js that I put together in about 15 minutes based on your problem description. For demonstration purposes, it just returns the predicate names; to use it, you'd change it to return a function that takes the predicates object and invokes the appropriate predicate, but hopefully this is enough to give you an idea of one possible approach.
OrExpression
= head:AndExpression tail:(_ ("OR") _ AndExpression)*
return tail.reduce(function(result, element) element[3];
, head);
AndExpression
= head:NotExpression tail:(_ ("AND") _ NotExpression)*
return tail.reduce(function(result, element)
return result && element[3];
, head);
NotExpression
= "NOT" _ expr:NotExpression return !expr;
/ "(" _ expr:OrExpression _ ")" return expr;
/ Predicate;
Predicate
= _ predicate:[A-Z]+ _ "(" _ arg:[A-Z]* _ ")"
return predicate.join('') + '(' + arg.join('') + ')';
_ "whitespace"
= [ tnr]*
edited Aug 17 at 21:46
answered Aug 17 at 21:37
Josh KelleyJosh Kelley
2471 silver badge9 bronze badges
2471 silver badge9 bronze badges
add a comment |
add a comment |
Thanks for contributing an answer to Information Security Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fsecurity.stackexchange.com%2fquestions%2f215219%2frunning-code-generated-in-realtime-in-javascript-with-eval%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
24
It's not just that eval is potentially insecure (which it is), it's also that solutions relying on
eval
are typically hard-to-follow, confusing, and impossible to debug when things break. There are nearly always solutions that are, in all ways, better.– Conor Mancone
Aug 13 at 12:52
21
I mean, I don't know your exact use-case, but I really doubt you have to go so far as building an entire language to make this happen. I suspect you're over thinking it. I've built a rules engines that allow the user to fully configure detailed conditions that have to be met before taking arbitrary actions, and did so without requiring either eval or a meta-language. In fact my rules engine sounds much more complicated. Things like: "If this is the customer id, and the status is one of these, and it was created more than X days but less than Y days ago, then send this email and this text
– Conor Mancone
Aug 13 at 13:53
4
I suppose I can't exactly break down a new system for you in comments, but effectively my answer is "I think you're using the wrong approach and should come up with a different solution". Eval is rarely the answer.
– Conor Mancone
Aug 13 at 13:59
18
I do wonder why your developers are not writing javascript straight away. Your syntax doesn't seem to have that much of a conciseness and readability advantage, but the regular expression compiler is very limited.
– Bergi
Aug 13 at 20:56
9
If all you are doing is to hide and show menus, why not use CSS selectors instead of Javascript?
– Ángel
Aug 13 at 22:24