Addon enqueued for rejection and I don't have a way to fix it

Add-ons addons.mozilla.org
#1

Good day,

I have a problem with your moderator team.

I have an extension (it is on the platform since October 2019 and has had no complaints from the users), which has some slightly obfuscated code (minification). All that code does is URL unhashing (to make them usable).

It is possibly breaking the copyright laws of my country, but the reason for rejection is not that thing. The reason is that the code is minified. The problem is that the code is not written by me (it’s authors are VK’s developers), so I cannot provide the source.

I provided at least 2 links on GitHub to one of your moderators with the code very similar to mine to prove that this code really does the URL unhashing for VK Audio.

But that was not accepted, I was told that if I am using a library it should be a release version and not modified. But it is not a library, and that code on GitHub is not a library too, it is part of other 2 projects (like extended VK API and some downloaders).

I know that asking to make some exception is maybe wrong, but honestly it is absurd what is going on now. I never changed that part of code since the original extension upload here (well, maybe only 1-2 times when they changed the hashing function on VK side), and now I should either remove the download audio feature or get banned completely.

What’s the point of this witch-hunting? I’m not doing anything malicious with that code. It is obvious if you read how and where it is called, at least. Also you can test my extension and its download current audio track feature directly on vk.com, I can even make an account for such test.

Here is my extension’s page: https://addons.mozilla.org/ru/firefox/addon/vk-plus-script/

#2 
function unmask(t) {
   "use strict";
   function i() {
      return window.wbopen && ~(window.open + "").indexOf("wbopen")
   }
   function o(t) {
      if (!i() && ~t.indexOf("audio_api_unavailable")) {
         var e = t.split("?extra=")[1].split("#")
         , o = "" === e[1] ? "" : a(e[1]);
         if (e = a(e[0]),
         "string" != typeof o || !e)
         return t;
         o = o ? o.split(String.fromCharCode(9)) : [];
         for (var s, r, n = o.length; n--; ) {
            if (r = o[n].split(String.fromCharCode(11)),
               s = r.splice(0, 1, e)[0],
               !l[s])
            return t;
            e = l[s].apply(null , r)
         }
         if (e && "http" === e.substr(0, 4)) return e
      }
      return t
   }
   function a(t) {
      if (!t || t.length % 4 == 1)
         return !1;
      for (var e, i, o = 0, a = 0, s = ""; i = t.charAt(a++); )
         i = r.indexOf(i),
         ~i && (e = o % 4 ? 64 * e + i : i,
         o++ % 4) && (s += String.fromCharCode(255 & e >> (-2 * o & 6)));+
      return s
   }
   function s(t, e) {
      var i = t.length, o = [];
      if (i) {
         var a = i;
         for (e = Math.abs(e); a--; )
            e = (i * (a + 1) ^ e + a) % i,
            o[a] = e
      }
      return o
   }
   var r = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMN0PQRSTUVWXYZO123456789+/="
   , l = {
      v: function(t) {
         return t.split("").reverse().join("")
      },
      r: function(t, e) {
         t = t.split("");
         for (var i, o = r + r, a = t.length; a--; )
            i = o.indexOf(t[a]),
            ~i && (t[a] = o.substr(i - e, 1));
         return t.join("")
      },
      s: function(t, e) {
         var i = t.length;
         if (i) {
            var o = s(t, e)
            , a = 0;
            for (t = t.split(""); ++a < i; )
               t[a] = t.splice(o[i - 1 - a], 1, t[a])[0];
            t = t.join("")
         }
         return t
      },
      i: function(t, e) {
         return l.s(t, e ^ vk.id)
      },
      x: function(t, e) {
         var i = [];
         return e = e.charCodeAt(0),
            each(t.split(""), function(t, o) {
               i.push(String.fromCharCode(o.charCodeAt(0) ^ e))
            }
            ),
         i.join("")
      }
   }
   return o(t)
}

Here is the code in question.

As for me it is obvious that the function unmask(), added to window object via the tag, when it is called like

ge('download_audio_button').setAttribute('data-url', ap && ap._currentAudio ? unmask(ap._currentAudio[2]).split('?')[0] : '')

in the other injected tag is completely safe.

Also it can be easily type-checked that the input and output of all the internal functions inside the closure is String. And neither window.location nor document.cookie is accessed from anywhere inside the code, also no eval() or Function('some string') is used, so no possible side effects. In fact, nothing is even assigned to window or document, so also no malicious co-operation with other addons possibly installed pair-to-pair with mine.

#3

You can’t have minified code in your codebase that you don’t understand :smiley:. This is something that could get you fired if you did it in a normal IT company.

And imagine how the reviewer must feel - he has to suspect all submissions are malicious and that the attacker will try some social engineering techniques to gain his trust. If he would let you submit a minified code, he would fail his job.

I would recommend reading some deep analysis of some of the known malware to see how it works.

Also saying that it doesn’t have eval is not enough for the language like JavaScript where you can dynamically build the code… have you seen JSFuck? :smiley:

Anyway, if the problematic part is the this piece of code you pasted here, why not simply un-minify it? First use some online un-minifier to fix the code structure and then go through it line by line and fix the variable names.

#4

Similarly, I think if you had the unminified source (as you seem to have provided above), minified it yourself, and provided the build scripts they would accept that too so long as they can see how you got from start to finish - did you try that route with the moderator yet?

#5

I understand that it is a symmetrical encryption code, I don’t need to know the details of this implementation.

No, I haven’t seen JSFuck, that’s quite interesting idea, but obviously this code is not such case.

Again, what about type checks? It is quite easy to see this code is safe.

#6

Sorry, I don’t have the source. The links that I provided to the moderator are to another minified code nearly identical to mine, that does the same decyphering logic.

#7

And 2 more things:

  1. Why not to perform the test on VK site to see how this code is invoked (to see that it just returns a link to MP3)?
  2. Why this minified code was not a problem for 2 years, something changed in security policies?
#8

This happens all the time and it happened to me as well (reviewer disabled multiple last versions because of some small issues).
They don’t have enough reviewers to manually check everything right away so I guess some addons will be manually checked only after they become more popular (or maybe after someone reports it).
Anyway, it’s too late now, all you can do is comply with the policies…

That sounds crazy complicated and not reliable enough… Remember, the reviewer is someone like you or me - a lazy developer (:D) with deadlines and 1000 other important things to do that doesn’t have time to argue or follow complicated steps. It’s hard enough for them to check normal code for suspicious behavior so if you send them minified code then it’s just too much.

Don’t take it personally, sh*t happens sometimes.

1 Like
#9

Thanks for your understanding and support :slight_smile:

Though I disagree on the point that a test which I suggested is more complicated than reading through normal (!), not minified code (the code is large, and the test takes just 3-4 minutes at worst), but I agree that there’s nothing we can do with the rules and reviewers’ decisions, unfortunately.

Again, I doubt if the suggestion to “unminify code manually” will help, otherwise I think the reviewer would advise me to do so.

But I’ll try it, whatever :slight_smile:

1 Like
#10

This, by itself, directly prevents the Add-On from being signed by Mozilla in any capacity.

What you could do is take one of those instances of that code, and, though you can’t import it as a library due to its not having fixed releases, and you can’t paste opaque gobs of foreign code and have it pass muster, instead just re-implement the algorithm from one of those other open-source implementations into your codebase. If it’s under 40 lines, it should be less than an hour’s effort to simply retype the code with better variable names, the same indentation as the rest of your code base, maybe a few comments for the reviewer.

It’s not just about complexity (though I’d argue that signing up for a Russian social media service is quite a bit of overhead), but also, as stated by juraj, about reliability. Reading over the code is going to be infinitely more reliable than saying “if you go to a certain website on a certain date nothing happens”. It’s not about the Add-On behaving during review; it’s about it not having any undesired behavior, and looking at a single white swan can by no means prove a negative.

Indeed: it would be far better (and, in fact, it is required) to just provide the actual, non-minified, source code that produced the files you’re trying to publish for Firefox users to install. If you want to publish an Add-On that contains closed-source gobs of executable code that you found on the Internet, you won’t get any signatures under the current policy.

It’d be a treat if you could “unminify” this closed-source code to the reviewer’s satisfaction, but not to be expected, by any means. Even Amazon must disclose the entire source code (including all libraries) of their proprietary Add-On to Mozilla Reviewers.

1 Like
#11

I changed some variable names and added 1-2 comments, but he ignored my link for some reason. The indentation was already correct, as in the other codebase of the extension.

Sorry, what do you mean by “that site on that date”? My addon works only with that site, which can be verified by the permissions in the manifest. And it is clearly seen by the code that it does not read the current time in any place.

Again, what is the reason for re-implementing the code that just works? It is not minified to the point where it is meaningless like JSFuck or something that came from the obfuscator.

I would ask to change the reviewer, if it is possible, because the replies come once in 7-8 days, the version of the code with comments, better variable names and some codestyle changes that I sent via Pastebin was ignored, and the extension was blocked completely.

#12

This is the version where I changed some var names and got rid of commas between the expressions: https://pastebin.com/6mE2VXvS

If you think that is not enough, I would be glad to see any improvements.

And also I don’t have an idea how you can tell that the “source” or “unminified” code provided by me is really equivalent to the minified version in terms of behavior. There are too many minifiers out there on the net, so the output will be very different.

#13

So, any ideas about how to improve that version more? Did I do it badly?