CodeSOD: Last One In

A lesson that everyone learns at some point is "don't write your own authentication code." Authentication, like encryption, and like dates, is incredibly complex and has all sorts of ways you can subtly mess it up and not realize your mistake.

Take, for example, this code from Christopher. His peer wrote this code, added a single test record to the database, saw that it worked, and called it a day.

public void Login(string userName, string hashValue){ SqlDataReader dr = getUsers(); while (dr.Read()) { if (dr["userName"].ToString() == userName && dr["hashPassword"].ToString() == hashValue) { this.authenticated = true; } else { this.authenticated = false; } }}

Christopher spotted the core problem, and tried to explain it to the developer. When it wasn't sinking in, he quickly whipped up a script to add tens of thousands of users to the table, keeping the developer's username as the last row in the table. They could still log in, but logging in took much longer.

"Maybe I need to add an index?" was their response.

Christopher added one more row to the database, and now the developer was no longer able to log in.

"I don't understand? Why did it stop working?"

"You're looking at every row, and setting this.authenticated for each one. So the only one that ends up counting is the last one."

The lightbulb finally went on, but it wasn't a particularly bright one. "Oh, right, I should add a break if it matches!"

"Or," Christopher said, "let me tell you about a little thing called a WHERE clause..."

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!