CodeSOD: Rube Goldberg's Password Generator

One of the well-known rules of life is that the most straightforward solution is usually the best solution. Obviously it's not always possible to "keep it simple, stupid," but one should aim to make their creations as self-explanatory and to-the-point as possible- otherwise it's easy to end up with a nightmare in terms of both maintainability and performance.

Some people, however, have chosen to defy that rule. One of them was Rube Goldberg. This engineer turned cartoonist became famous for inventing ridiculously complex contraptions to achieve the simplest tasks. And while Mr. Goldberg passed away in 1970, the concept of a "Rube Goldberg machine" outlived him, showing up in hundreds of cartoons, events, and comedy movies.

And, as Matt R. learned, it also made its way into his codebase. While refactoring and rewriting a 32,000-line long file, he came across this incredible machine:

private string GeneratePassword(){ string guid = Guid.NewGuid().ToString().ToUpper(); while (guid.Contains("-")) guid = guid.Remove(guid.IndexOf("-"), 1); string guidInt = ""; int i = 0; char c; while (i < guid.Length) { c = guid[i]; if ((c < '0') || (c > '9')) { ++i; continue; } guidInt += c.ToString(); ++i; } int seed = 0; if (guidInt != "") { try { guidInt = guidInt.PadRight(9, '0').Substring(0, 9); seed = System.Convert.ToInt32(guidInt); } catch { } } Random random = new Random(seed); string pwd = ""; while (pwd.Length <= 8) { c = (char)random.Next(48, 123); if ((c < 48) || ((c > 57) && (c < 65)) || ((c > 90) && (c < 97)) || (c > 122)) continue; pwd += c.ToString(); } // 05.08.2014 sometimes the PW has no number in it and that is required, so add it here if needed i = 0 ; bool bNumberFound = false; while( i < pwd.Length ) { char x = System.Convert.ToChar(pwd.Substring(i,1)); if (Char.IsNumber(x)) { bNumberFound = true; break ; } i++; } if (!bNumberFound) { pwd = pwd + "1"; } return pwd;}

Tracing the code, we see that first it generates a GUID and turns it into uppercase. In any normal code, this would merely be a warning sign. GUIDs aren't a good source of randomness, and as such don't belong anywhere near a function for generating random passwords. In this code, however, it's more of a sinister omen of things to come...

In the next step, all dashes are removed from the GUID. Of course, using String.Replace would be a simple solution, so instead, the programmer opted for another one: the while loop looks for a single dash, then if one is found, the string is searched again to determine where that dash is, and finally it's removed from the string, shifting all the following characters to the left. It's a good thing GUIDs are relatively short.

After that, the real fun begins. The GUID is used to seed a random number generator (since seeding with current time is, again, a simple solution). How does one do that? Well, of course, by extracting every numeric character from the GUID, collectng them into a string, padding the string with zeroes, trimming it to nine digits, converting the string to an integer, and finally using that to seed the generator. Whew! Oh, and if the conversion fails for some reason, or if the GUID contains no digits, you get a seed of 0.

After all that, finally a 9-character password is generated. Occasionally, however, it will fail to contain any numbers, so the code just checks whether that's the case, and slaps a "1" at the end if so- rendering the attempt of increasing password entropy entirely pointless.

With all the effort put into the solution, it's hard to know whether to be amused or terrified. Personally, I think this code warrants at least a commemorative real-life Rube Goldberg machine- preferably ending with an anvil hanging above the developer's cubicle.

[Advertisement] BuildMaster is more than just an automation tool: it brings together the people, process, and practices that allow teams to deliver software rapidly, reliably, and responsibly. And it's incredibly easy to get started; download now and use the built-in tutorials and wizards to get your builds and/or deploys automated!