BestBuy, worst JS
Monday, 4. February 2008, 14:24:32
Something tells me that BestBuy redesigned their home page last week and replaced the old OpenCube script with something more modern - looks exactly the same as the old menu but now it's a simple unordered list in the markup itself, hover effects provided by CSS. So far so good!
Looking around a bit, there is plenty of other code that could do with some modernising. Here's a snippet of the function handleEnterKeyPress (one of many enter-key related functions):
Wow, using arguments.callee.caller and the arguments property on function objects to look up the pressed key - would be more than a little cleaner to pass the event object to the function as an argument.. And the sniffing is ugly.
It seems somebody needs to learn about JavaScript's bracket syntax -
Having good names for functions can be a good idea too:
They have one of those addEventListener/addEvent/set event handler directly kludges.. but with a twist:
Is it just me, or is the window[fnc](); statement completely broken? The fnc argument to this function is supposedly a function object, not a string. So doing window[fnc] will decompile the function and look for window['function (){ ..code here.. }'] which is not likely to return anything. Somebody *still* needs to learn about JavaScript bracket notation (well, mr. dwl in this case).
Finally, looking at this scares me a bit:
I don't know much ActiveX but it looks like they use it for in-store information kiosks. If you find an XSS issue in their kiosk site or can use DNS poisoning to serve some JS that shouldn't be running there it looks like you can take over the machines completely with shell scripting from your JS! Wow..
Looking around a bit, there is plenty of other code that could do with some modernising. Here's a snippet of the function handleEnterKeyPress (one of many enter-key related functions):
if(navigator.appName == "Netscape")
{
if(arguments.callee.caller.arguments[0].which == 13)Wow, using arguments.callee.caller and the arguments property on function objects to look up the pressed key - would be more than a little cleaner to pass the event object to the function as an argument.. And the sniffing is ugly.
It seems somebody needs to learn about JavaScript's bracket syntax -
if (tabnumber==1)
{
document.tab1.src = imgServer + locale + "/images/global/misc/pdptabs/tab_prodspecs_over.gif";
}
if (tabnumber==2)
{
document.tab2.src = imgServer + locale + "/images/global/misc/pdptabs/tab_accessories_over.gif"; and the 5 other if clauses would be much nicer as document['tab'+tabnumber].src='...'.Having good names for functions can be a good idea too:
/**
* test for https:
* return true for secure
* false for standard http
* jm 2006/10/24
*/
function getProtocol(){
var isSecure = (window.location.protocol == "https:") ? true: false;
return isSecure;
}SourceThey have one of those addEventListener/addEvent/set event handler directly kludges.. but with a twist:
/**
* takes function call
* delays execution until full page load
* dwl 2006/09/28
*/
function addOnloadEvent(fnc){
if ( typeof window.addEventListener != "undefined" )
window.addEventListener( "load", fnc, false );
else if ( typeof window.attachEvent != "undefined" ) {
window.attachEvent( "onload", fnc );
}
else {
if ( window.onload != null ) {
var oldOnload = window.onload;
window.onload = function ( e ) {
oldOnload( e );
window[fnc]();
};
}
else
window.onload = fnc;
}
}Is it just me, or is the window[fnc](); statement completely broken? The fnc argument to this function is supposedly a function object, not a string. So doing window[fnc] will decompile the function and look for window['function (){ ..code here.. }'] which is not likely to return anything. Somebody *still* needs to learn about JavaScript bracket notation (well, mr. dwl in this case).
Finally, looking at this scares me a bit:
function getSnum(){var WshShell=new ActiveXObject("WScript.Shell");var sName=WshShell.ExpandEnvironmentStrings("%LOCATIONNUM%");return sName;}I don't know much ActiveX but it looks like they use it for in-store information kiosks. If you find an XSS issue in their kiosk site or can use DNS poisoning to serve some JS that shouldn't be running there it looks like you can take over the machines completely with shell scripting from your JS! Wow..
You must be logged in to write a comment. if you're not a registered member, please 
By shadowk, # 4. February 2008, 15:32:20
function validatePasswd(fn) {
var fval;
var fvalUID;
var userName="userName";
eval("fval = " + "document.Main." + fn + ".value");
eval("fvalUID = " + "document.Main."+userName+".value");
var inValid = false;
var x1 = /[a-zA-Z]/; // only alphanumerics, and length 6-10
var x2 = /[A-Z]/; // a letter present
var x3 = /\d/; // a digit present
var x4 = /[.@_-]/; // a special character present
var x5 = /[^a-zA-z0-9._@-]/;
var x6 = /\^/;
var x7 = /\\/;
var Ufval=fval.toUpperCase();
var UfvalUID = fvalUID.toUpperCase();
if(Ufval==UfvalUID){
inValid=true;
}
if(!fval.match(x1) || !fval.match(x3) || fval.match(x5) || fval.match(x6) || fval.match(x7) || fval.length<6 || fval.length>25){
inValid = true;
}
if(fn == "CPassword" && fval != document.Main.password.value) {
inValid = true;
}
return inValid;
}
In particular, look at lines 4 and 5 of that function.
By wycats, # 4. February 2008, 21:22:49
By hallvors, # 4. February 2008, 22:41:41
For example, for email validation it used
var validEmail = new RegExp("@", "i");
Somehow using regular expressions just to check for an @ symbol seemed like a little bit of overkill, I was also slightly puzzled as to what the "i" flag was hoping to achieve here ;-)
Of course there were plenty of other fun problems like the Javascript validation (in most cases they didn't bother with any server side validation) allowing postcodes to be entered in a format that would later completely break another part of the application, and telling users that - was an allowed character in their username, only to reject any username containing a - (with no explanation as to why their registration had failed)
The sad thing was their HTML was probably worse than the Javscript...
By catbert303, # 5. February 2008, 10:31:58
You'd have say that many sites are just barely working by the skin of their teeth!
By Andrew Gregory, # 5. February 2008, 13:21:26