宣伝。日経LinuxにてLinuxの基礎?を紹介する漫画「シス管系女子」を連載させていただいています。 以下の特設サイトにて、単行本まんがでわかるLinux シス管系女子の試し読みが可能!
This is an translated version of my another entry, evalが危険でそれ以外の方法が安全だと思ってる人へ . This is an objection for the entry: Five wrong reasons to use eval() in an extension written by Wladimir Palant.
Recently I updated Source Viewer Tab and Tree Style Tab. However, when I uploaded the latest version to AMO, both files were retained in the sandbox, never made public by AMO editors. They said with one mouth: "Your add-on uses the 'eval' function unnecessarily, which is something we normally don't accept. There are many reasons *not* to use 'eval', and also simple alternatives to using it. You can read more about it here: http://adblockplus.org/blog/five-wrong-reasons-to-use-eval-in-an-extension"
I know the point of their blames very well, because I read and translated the entry to Japanese for my understanding. When I got a rejection about another addon, I got just same comment.
Now I have an objection about indiscriminate rejections by "too many eval()
s" like this.
First of all, there is no "wrong" use of eval in any JavaScript codes (not only my addons). All of evals are "right" if it does its own work correctly.
For example, by the HTML specification, <em> is defined as "means emphasizing", "cannot appear in <title>", and so on. If you write <em> against the definitions, your HTML will be "invalid".
How about eval? 15.1.2.1 eval (x)
in the ECMAScript specification describes about the function. It just says: runs the specified string as a script. There is no cautionary statement about the purpose of using eval.
There is no "wrong" use, it is just "dangerous" use. There are many usecases: some is high-risk, some is low-risk. But, they all evals are "right" use if it works as required by the spec. So, I think that it is a suitable subject for the entry: "Five dangerous use of eval()
in an extension".
Risks of eval have to be managed, in other words, risk-managed evals are useful tool to extend Firefox. For end users, I think some eval should be allowed if it is safe enough in the case. "Eval? No, no, it is evil function! Don't use it anyway!" - I think that is slapdash work.
When you get 5-minute recipes via HTTP, do you untrust it anyway? Because it is insecure protocol, you cannot trust any information without SSL? Note, VeriSign certifies that the website you are accessing via HTTPS is surely hosted by the author, but he doesn't certify that the author is not evil. If you cannot trust any people via internet, you have to live offline - it's a internet phobia. In many cases you maybe trust them anyway if it is low-risk.
In the entry, he negates any eval without considering about its risk if it is out of the "valid uses". I think it is just a eval phobia.
I agree to blame some cases of 5 cases in the entry. The 1st (parsing JSON), the 2nd (accessing to properties dynamically), and the 4th (running event handlers of HTML or XUL elements) - those three cases must be blamed because they possibly run untrusted scripts from the Web as privileged. This is just same to caution about SQL injections for web apps.
I totally agree this topic. I also think any developer must be careful to make eval isolated from the Web. If my addons are rejected because they have this security issue, I totally back the judgment up.
So, how about my cases?
Both addons, Source Viewer Tab and Tree Style Tab, use eval only for injecting codes to functions in Firefox (and other addons). They never get codes from the Web. If editors decided to reject those addons by the reason, like "This is dangerous because it includes eval! Eval can run codes from the Web!", then it means: they reviewed addons without reading codes, and just checks warnings by the "not perfect" validator.
If so, I cannot agree to the decision. We shouldn't overestimate the power of the validator on the AMO system, because it can detect only a few patterns which are defined as "dangerous". It doesn't parse the JavaScript, it only reports the matching result about simple regular expressions, it cannot detect dangerous codes from obfuscated programs.
You have to judge case by case whether the eval is danger or not. Same about XBL hacks, CSS hacks, replacing of functions, and others. "Yes, this doesn't use eval. I accept it." "Oh, this uses eval. This is wrong. I reject it." Such a rash judgment possibly jeopardize people who download addons from AMO. I believe that we should review sources by our eyes to detect dangerous code cleverly obfuscated, even if it take time.
All of evals in Source Viewer Tab and Tree Style Tab are used to inject codes to functions of Firefox, it is the 5th case described in the entry. In the entry, he blames this use because:
And, he said that we should use "less dangerous" ways like:
func.apply(this, arguments)
.Object.watch()
.In other words, in the entry he said that "those ways are safer than eval".
I think it is an unfair suggestion. If you talk about risk of those cases, you should think other risks too, which are not listed in the entry. We also would like to avoid eval if at all possible. However, in our cases eval is the way really better than others.
Methods to extend Firefox can be sorted as:
Object.watch()
or other ways.One addon can use multiple ways above, but we can call it "safe" addon if it uses only "stable APIs". Otherwise they are possibly unstable. Any addon using unstable ways possibly breaks Firefox and other addons. Themes also.
He said that the "less dangerous" way is safer than eval. But, actually it possibly isn't. For example, I tried to watch the toggling of the Sidebar in Firefox 3.6, by document.getElementById("sidebar-box").watch("hidden", ...)
. Then it breaks the original behavior of XUL element - the Sidebar was never toggled. (So I had to use an event listener for the "DOMAttrModified" generic event.)
Another case, if you wrap the original function by a new function, any other addon which aimed to inject codes to the original function will never work. Hmm, "Because it is evil. Evil addons using eval must die!" Don't say that. It is the fact, you didn't take care about people who use addons with eval.
By the way, there are features which cannot be implemented without injecting codes into existing functions. Dorando told about those addons in the comments to the entry. The conclusion of the entry is "don't do it, you should not do it". However, if you cannot explain how dangerous eval is than others, it is just a tautology: "you should not do it, because it is wrong use of eval." and "it is wrong use of eval, because you should not do it."
On this point, both methods (eval and the "less dangerous" way) have same risk. He said that eval will never work if the definition of the function changed. However, the "less dangerous" way will collapse too if the original function is renamed. It is just a bias that functions will be never renamed in security updates. There is no "rule" like this.
Moreover, if you wrapped two functions A and B, expecting they work as "A-mod => B-mod", the hack will break down if only one mate changed by security fix. We cannot imagine how terrible thing caused is. Not only updating of Firefox, but conflicting to other addons too.
Anyway, we addon developers must catch up changes in any update of Firefox. It is the common issue for any addon which uses unstable ways to extend Firefox. I think blaming like "how terrible eval is than wrapping original functions" is nonsense. More importantly, I believe that we should talk about providing failovers for unexpected matters.
To put it bluntly, any addon is just a "dynamic patch". If you use Linux, you possibly download patches from ML or other places, apply it to the source, and build binaries. It is not uncommon sight. And, it is also important note in those cases: Don't apply the patch to any revision different from the base of it. Using addons on unverified versions of Firefox is just same to that.
If we use eval hack, only "codes should be replaced" and "codes should be injected" will appear in the source.
Generally reading source codes like this is certainly hard, because we have to refer the codes of the function which is being overwritten. Instead, there are less codes and sources become compact. If it is very simple like replacing from "width" to "height", it will be human readable enough. So know-how to keep maintainability and compatibility is important for this hack.
Even if you choose the way to wrap original functions, only fragmentary codes will appear in the source too. "Pre-operations", "post-operations", "wrapping functions", "watching functions for property changes", and others. Moreover, if you cannot get the result of operations by the original function as its returned value, eventually you have to know well how the function work.
In this case, if you want to implement a feature which require injection of some codes into the logic of an existing function, you need to copy the complete codes of the original function into your addon (of course you have to modify it.) Then, too many codes are possibly duplicated, so:
This is common to the topic about maintainability. AMO editors finally have to review addons with reading of their sources. If there are tricky codes or too many/long lines, then the cost of reading source codes increases. On this point, both eval hacks and wrapping hacks are similarly tricky.
Although if updates of addons are always rejected by the reason: "there is any eval", then, it becomes a new common practice: writing codes without eval anyway, with tricky hacks, even if they become hard to read. Otherwise the update won't be accepted. It is also a tautology: "we should not use eval, because they never accept codes including eval." and "they don't accept eval hacks because we should not use it."
As I described, I think evals in Source Viewer Tab and Tree Style Tab are not unnecessarily. In other words I believe that those evals are the best way to implement the feature with keeping compatibility. I think those evals are not absolutely dangerous than the "less dangerous" ways described in the entry, and I also think there are no alternative ways to do it.
How many people who says "eval is dangerous than others" actually read and reading the source codes of Mozilla products? Trying to extend Firefox even if there is no stable API? Catching up updates of Firefox constantly? Trying to solve compatibility problems with other addons? I'm feeling blue.
Only XPCOM components are really stable APIs, which are marked "FROZEN" in their IDL files. Otherwise they are absolutely unstable. There are only a few "FROZEN" APIs, preferences I/O, W3C DOM, etc. JavaScript functions in the browser window are very unstable. Even the FUEL - which was the component advertised as providing stable APIs independent from Firefox versions - is possibly removed from the next major release of Firefox. Yes, this is the Mozilla. I think it is just a kidding: "eval is dangerous, wrapping original functions by new functions with the name same to originals is safer than it."
In conclusion, I think it is unjust treatment that addons are retained in the sandbox because "they have evals".
To reject spams, you have to answer a quiz to comment to this entry. "今の日本の首相の名字(ひらがなで回答)" means: the family name of the current prime minister of Japan, in "hiragana". You'll see the answer from wikipedia (Japanese version of the page).
の末尾に2020年11月30日時点の日本の首相のファミリーネーム(ローマ字で回答)を繋げて下さい。例えば「noda」なら、「2010-02-08_eval-en.trackbacknoda」です。これは機械的なトラックバックスパムを防止するための措置です。
writeback message: Ready to post a comment.