I've posted about it in other forums, and now that I've discovered the wonderful world of WTF, I must share it here as well.

One of our customers had a requirement that their users not be allowed to use really dumb PINs like 1111, 1234, 4321, etc... can't be same number 4 times, can't be ascending or descending runs.  Here's the lovely bit of ECMAScript that our friends offshore cooked up to satisfy the requirement:

 
          var index = 0;
          for( ; index < 4 ; )
            {
            current = ToInteger(new_pin.substring(index, index));
            if(index == 0)
              previous = current;
            else
              {
              if(all_the_same == 1 && previous != current)
                all_the_same = 0;

              if(index = 1)
                direction = previous - current;
              if(((direction != 1) || (previous != (current - 1)) && ((direction != -1) || (previous !== (current + 1))))
                  consecutive = 0;
              }
            if(consecutive == 0 && all_the_same == 0)
              {
              PinIsCompliant = ToString(true);
              break;
              }
            index ++;
            }

 

Not only is that some whacky use of a for loop, but the thing got false negatives for stuff like 1357 or 0369

There are only 10,000 pins anyway, and they want to limit the keyspace more...hmm...

Just because I'm wondering now, how would the rest of you done it?

Personally, I would have just had a lookup table or a local hash and pulled a:

 

if (badPins.contains(newPin)) {
  //dont use this pin
}
 

Can anyone explain how ToString(true) differs from "true"?

In any event, why would one call the primitive operators?  xxxxx.toString() and parseInt() are perfectly good.

Apparently they've also never discovered charAt(), and I don't even think substring(i,i) returns anything but an empty, since the definition says "starting at the first argument, up to, but not including, the index of the second argument."  I'll have to test that, though.

Hellz99:

Just because I'm wondering now, how would the rest of you done it?

Personally, I would have just had a lookup table or a local hash and pulled a:

 

if (badPins.contains(newPin)) {
  //dont use this pin
}
 

I disagree! With integers, there are 10 of the same (0000...9999) and 14 "runs" (0123, 1234, 2345, ... 6789, 9876, 8765, ..., 3210) - for a total of 24 out of 10,000 items that must be disallowed.

This requires a much more enterprisey solution, in case someone changes it from integers to doubles. A mere hash won't do. Perhaps a series of inverted logic if-else's. Perhaps some XML. Perhaps files named for each of the disallowed combinations (then we get to use FILE_NOT_FOUND in a meaningful way!) Think of the possibilities... *smirks*

 

You're right progguy

current = ToInteger(new_pin.substring(index, index)); 

is just going to return the charAt(index) and then turn it into an int. 

I guess if the original scope was the algorithm handled pins of larger num of digits, then maybe I can understand why you would do it with the for-loop....but not if the constraint was always going to be 4 digit. 

I don't see any reason to complicate it any further than matching against a list of bad pins.  No, it's not difficult to define the bad pins algorithmically, but there's only 26 bad pins out of 10,000 possible (and that's if you consider 0 to be below 1 AND above 9 in sequence).

It's relatively futureproof, too -- if the requirements get changed so that '1337' gets banned for being so 'leet, you just have to add that pin to the list, instead of changing the validation logic to be "if isAllSameNumber or isAscendingSeq or isDescendingSeq or isLeet"...

 

Apparently they've also never discovered charAt(), and I don't even think substring(i,i) returns anything but an empty, since the definition says "starting at the first argument, up to, but not including, the index of the second argument."  I'll have to test that, though.

Yup, substring(a,b) is (from, up-to-but-not-excluding), or in range notation: [a,b)

charAt is more like it. Although frankly my solution for a similar problem is a bit more of a WTF.

function reduce (old_char, in_char, o_class, c_class) {
        var delta;
        var p_off;
        switch (c_class) {
                case 'lower':
                        in_char = in_char.toUpperCase();
                        old_char = old_char.toUpperCase();
                case 'upper':
                        if(o_class != 'upper' && o_class != 'lower') { return false; }
                        delta = Math.abs(in_char.charCodeAt(0) - old_char.charCodeAt(0));
                        return (delta < 2 || delta > 24);

                case 'punct':
                        p_off = ")!@#$%^&*(".indexOf(in_char);  //Punctuation for each keyboard number in ASCII order
                        if(p_off < 0) { return false; }
                        in_char = String.fromCharCode(p_off + "0".charCodeAt(0)); //Add ASCII '0' back to it
                case 'digit':
                        if(o_class == 'punct') {
                                p_off = ")!@#$%^&*(".indexOf(old_char);
                                if(p_off < 0) { return false; }
                                old_char = String.fromCharCode(p_off + "0".charCodeAt(0));
                        } else if (o_class != 'digit') { return false; }
                        delta = Math.abs(in_char.charCodeAt(0) - old_char.charCodeAt(0));
                        return (delta < 2 || delta > 8);

                default:
                        return false;
        }
}

It's designed to be called from a loop that iterates over the password. It returns true when consequetive letters are "next" to each other (by one or two). If I had more time, I would have also had it check keyboard order (QWERTY only) but I ran out of time to play with it.

What happens when the length of the pin changes?

 I'm not really familar with how regex works in your language, but in perl I would start with...

 

perl -w -e "$i=9012; if('1234567890123456789' =~ m/$i/){ print $i;}"

and

perl -w -e "$i=2109; if('09876543210987654321' =~ m/$i/){ print $i;}"

 

or something similar with Instr if that's what you have...
 

I'd try this ( ruby ) 

def checkpin pin
   /^[0-9]{4}$/.match(pin) && !/^(.)(\1)$/.match(pin) && !"0123456789 9876543210".include?(pin)
end

I guess it's another case of unnecessary generalization. You never now, one day we'll count in hexadecimal, and you'll see who gets the last laugh !

I'd post my fix if somebody hadn't brought our VOB down at 4:30.....  Basically took the same approach they did though (minus the brokenness and hideous misuse of a for loop, of course).  If diff = dig[n]-dig[n+1] is the same for all digits, AND diff is in {-1, 0, 1} (the part they ignored), then PIN is invalid.

The regular expressions would have been an excellent solution though.  Don't think ECMAScript actually supports them, but we have other frameworks setup that would allow for us to do it fairly easy.  And at that point it'd be a lot easier to enforce other pin rules in the future.

 As to the number of digits, I think there was a written statement from the customer saying they always wanted 4 digits, but of course that'd be subject to change...we actually allow them to configure the maximum, so i probably would have thought to use that when i rewrote it.  Don't remember though.
 

. 

Duh. In fact there is not much differences between ECMAScript and Javascript, AFAIK. Javascript accesses different objects while in a browser, which makes sense since their purpose is to be embedded. Still, I'm pretty sure regexp is part of the core of both.

 This should work :

function checkpin(pin){
return pin.match(/^[0-9]{4}$/) && !pin.match(/^(.)(\1)$/) && "0123456789 9876543210".indexOf(pin) == -1;
};

Benanov:

There are only 10,000 pins anyway, and they want to limit the keyspace more...hmm...

This is the first thing that struck me, and you've put it far more succinctly then I would have.

Surely this doesn't have a positive impact on security?

As much as a WTF that code is, the requirement itself is also a pretty big WTF. 

Edit: pretty big huge

Shouldn't !pin.match(/^(.)(\1)$/) be !pin.match(/^(.)\1{3}/) ?

The four digit test is simply (n mod 1111) != 0 and the digit run test is ((n - 123) mod 1111) != 0.

pinkduck:

Shouldn't !pin.match(/^(.)(\1)$/) be !pin.match(/^(.)\1{3}/) ?

Whoops. Yep, 3 times and the parenthesis is not necessary.

TGV:

The four digit test is simply (n mod 1111) != 0 and the digit run test is ((n - 123) mod 1111) != 0.

(0xABCD - 0x123) % 0x1111 == 0

Great, you even solved the "one day we might wake up having three more fingers on each hand" problem :D

function checkpin(pin){
return pin.match(/^[0-9]{4}$/) && pin % 1111 != 0 && (pin - 123) % 1111 != 0 && (pin - 3210) % 1111 != 0 ;
};

Talking about newbees focusing on number representation rather than number properties, I'd better eat my own dog food :$

How about:

  var valid = false
  var diff = 0
  for(i=0; i<3; i++)
  {
    var newdiff = new_pin.charCodeAt(i) - new_pin.charCodeAt(i+1)

    if(Math.abs(newdiff) > 1)
    {
      valid = true
      break
    }

    if((i != 0) && diff != newdiff)
    { 
      valid = true
      break
    }

    diff = newdiff
  }

Hellz99:

Just because I'm wondering now, how would the rest of you done it?

Personally, I would have just had a lookup table or a local hash and pulled a:

 

if (badPins.contains(newPin)) {
  //dont use this pin
}
 

Might be the easiest way to do it.

pinIsCompliant =  ("0000x1111x2222x3333x4444x5555x6666x7777x8888x9999x0123x1234x2345x3456x4567x5678x6789x3210x4321x5432x6543x7654x8765x9876".indexOf(pin) ==-1);

ammoQ:

Might be the easiest way to do it.

pinIsCompliant =  ("0000x1111x2222x3333x4444x5555x6666x7777x8888x9999x0123x1234x2345x3456x4567x5678x6789x3210x4321x5432x6543x7654x8765x9876".indexOf(pin) ==-1);

That gets my vote! 

Hellz99:

Just because I'm wondering now, how would the rest of you done it?

Personally, since even 0369 is invalid because it is considered a run yet the numbers are not 1 off each other we simply need to see if we have a repeating pattern of difference. 

For a string 1 - n characters in length

Difference = Char(x) - char(x+1) 

If Difference <> PastDifference

    It is a good pin

Increment to next character.

If you never get to the good pin code it is bad.  This covers things like 1111 and 1234 and also 2468 and 0369.

The one thing this doesn't catch is layout runs such as things straight down the middle of the keypad 8520.
 

TGV:

The four digit test is simply (n mod 1111) != 0 and the digit run test is ((n - 123) mod 1111) != 0.

You win.

To those of you talking about limiting the number of PINs, you've gotta realize they're taking out 24 from 10000.  They hang up on the guy after 3 failed attempts, so if the hacker assumes you're stupid, he's gotta make 8 phone calls to brute force you.  With this rule in place, he's got to make 3326 calls.

// The real tragedy is that they don't ever permanently lock the guy's account, so making 3326 calls will actually work....but I'm sure they've got some other mechanisms in place to prevent/detect that sort of thing.

Here's my solution: 

          if(new_pin.length == 4){
            for( index=0 ; index &lt; 4; index++){
              current = parseInt(new_pin.substring(index, index + 1), 10);
              if(index == 0){
                previous = current;
              }
              else if(index==1){
                direction = previous - current;
                if(Math.abs(direction) &gt; 1){
                  PinIsCompliant = 'true';
                  break;
                }
              }
              else if((previous - current) != direction){
                PinIsCompliant = 'true';
                break;
              }
             
              previous = current;
            }
          }
 Why does new_pin.substring(index, index + 1) do the same thing as new_pin.substring(index,index) ?

You are all missing the point the OP made right at the end, he wants to also limit runs like 0369.  This will be more than 24 pins total, but not much more.

It isn't a change by 0 or 1 he is looking for, it is a change by a consistent number whether the change is by 0, 1, 2 or 3 so even 0246 and 1357 would be invalid.

My solution is the only one to satisfy that requirement so far.

 

While we're in there, let's remove the palindromic PINs.

Thanks for the tips on the [a,b) thing, too...I always forget that notation.  It also doesn't work when I use it in programs, but I think that's an entirely different problem.  Dunno, maybe it works in Perl.... ;)

KattMan:

You are all missing the point the OP made right at the end, he wants to also limit runs like 0369.  This will be more than 24 pins total, but not much more.

It isn't a change by 0 or 1 he is looking for, it is a change by a consistent number whether the change is by 0, 1, 2 or 3 so even 0246 and 1357 would be invalid.

My solution is the only one to satisfy that requirement so far.

 

OK... 

pinIsCompliant =  ("0000x1111x2222x3333x4444x5555x6666x7777x8888x9999x0123x1234x2345x3456x4567x5678x6789x3210x4321x5432x6543x7654x8765x9876x0246x1357x2468x3579x0369x6420x7531x8642x9753x9630".indexOf(pin) ==-1);