Skip to content
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

Unobfuscate plugin code #53

Closed
samuelmaddock opened this issue Nov 1, 2019 · 6 comments
Closed

Unobfuscate plugin code #53

samuelmaddock opened this issue Nov 1, 2019 · 6 comments

Comments

@samuelmaddock
Copy link

I was recently working with a client trying to implement a Content Security Policy (CSP) on their webpage. One of the first policies we added was to disallow unsafe-eval code. We soon ran into CSP violations caused by Adobe analytics plugins' obfuscated code.

As an example, here's the implementation of getPreviousValue

/* 
 * Plugin: getPreviousValue_v1.0 - return previous value of designated 
 * variable (requires split utility) 
 */ 
s.getPreviousValue=new Function("v","c","el","" 
+"var s=this,t=new Date,i,j,r='';t.setTime(t.getTime()+1800000);if(el" 
+"){if(s.events){i=s.split(el,',');j=s.split(s.events,',');for(x in i" 
+"){for(y in j){if(i[x]==j[y]){if(s.c_r(c)) r=s.c_r(c);v?s.c_w(c,v,t)" 
+":s.c_w(c,'no value',t);return r}}}}}else{if(s.c_r(c)) r=s.c_r(c);v?" 
+"s.c_w(c,v,t):s.c_w(c,'no value',t);return r}"); 
/* 
 * Utility Function: split v1.5 - split a string (JS 1.0 compatible) 
 */ 
s.split=new Function("l","d","" 
+"var i,x=0,a=new Array;while(l){i=l.indexOf(d);i=i>-1?i:l.length;a[x" 
+"++]=l.substring(0,i);l=l.substring(i+d.length);}return a"); 

I'm not sure what the reasoning is for the obfuscated code as it doesn't seem like something which should be a secret. Using new Function also limits the effectiveness of a CSP if users are allowing unsafe-eval.

I would suggest at the very least changing plugin snippets to not use eval. Here would be an equivalent for the s.split definition without using eval.

s.split = function (l,d) {
var i,x=0,a=new Array;while(l){i=l.indexOf(d);i=i>-1?i:l.length;a[x++]=l.substring(0,i);l=l.substring(i+d.length);}return a
}

relates to #17

@MicaPete
Copy link
Collaborator

@samuelmaddock - sorry about the delay - we are looking into this now.

@gigazelle
Copy link
Collaborator

Hi Samuel, this is an artifact from way back in the Omniture days when consulting helped with implementations and used these plugins extensively. With how easy it is to deobfuscate code and the open-source nature of the docs, it would indeed make more sense to open these up to improvements. At the same time, it acts as a deterrent against people screwing up the plugin, producing undesirable results.
I would love to open all of our plugins up, however, there might be some internal political discussions that may hinder this effort. I will kick off a discussion with our consulting team to discuss the potential to open our plugins up and make them editable and accessible.

@samuelmaddock
Copy link
Author

@gigazelle I can somewhat understand why obfuscation is used here, however, I believe the use of eval (via new Function) should be removed. There are very few reasons to use eval and it creates a problem for clients implementing a strict CSP.

I'd suggest using minified code such as the example I posted for the split plugin code.

@gigazelle
Copy link
Collaborator

Hi @samuelmaddock , I've spent the last couple weeks working with consulting and getting each plug-in up to snuff. You'll find that every plug-in should meet your requirements now. Thank you for your feedback!

https://docs.adobe.com/content/help/en/analytics/implementation/vars/plugins/impl-plugins.html

@samuelmaddock
Copy link
Author

Thanks @gigazelle! Your work is appreciated 🙂

I did find one instance which looks like was missed under the Installed Plug-ins section.

s.getQueryParam=new Function("qp","d",""
+"var s=this,v='',i,t;d=d?d:'';while(qp){i=qp.indexOf(',');i=i<0?qp.l"

@gigazelle
Copy link
Collaborator

Oh, yeah - that's a page that I'm working on today. It will be gone by tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants