Dilbert





The Daily WTF

CodeSOD: Doubly Encrypted Logins;

Providing authentication for your web-based APIs is both a challenging problem but also a largely solved problem. The hardest part is really just working your way through the various options, and from there it’s usually some variation on a drop-in component.

Done properly, it’s also client-agnostic. I can access the service from my browser, I can access it from a thick client, I can access it from cURL. Done incorrectly, well, you get what happened to Amira.

She was trying to pull some statistics from the backend, and couldn’t figure out how to authenticate. So she checked the front-end code to see how it authenticated:

crypt = new JSEncrypt();
crypt.setPublicKey('<removed>');
challenge = "<removed>";
function doChallengeResponse()
{
document.loginForm.password.value.replace(/&/g, '%26');
document.loginForm.password.value.replace(/\\+/g, '%2B');
document.loginForm.password.value = crypt.encrypt(document.loginForm.password.value);
document.loginForm.response.value = document.loginForm.password.value;
document.loginForm.password.value = '';
document.loginForm.submit();
}

On one hand, I want to guess that this code is very old, just based on the document.loginForm approach to interacting with DOM elements. On the other, JSEncrypt was first released in 2013, setting an upper limit on the age.

We send those credentials to the backend using a from submission, which our original developer decided needed some sanitization- all the & and + in the password are escaped, which… shouldn’t be necessary because the form should be doing a POST request, but also, we’ve encrypted the data.

Here’s my suspicion. This code is actually quite old. The original developer copied it from a circa 2005 StackOverflow post, and it didn’t include things like encryption or sending the form as a POST. Over the years, little things got added to it, one after the other, but the core mechanics never really changed.

Amira did check the history, and found that the previous version didn’t use encryption, and instead concatenated the password with the challenge variable, MD5 hashed the two, and sent that over the wire, which… well, almost kinda works, if you don't think about it too hard.

The good news is that once Amira figured out how to get the cookie she needed, the server never expires that token, so she’s free to keep it, send her requests via cURL, and never look at the web form again. She doesn’t have to worry about the cookie being compromised in transit, because the application uses and has always used SSL/TLS.

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


It's The End Of The Month As We Know It;

Calendario abril-junio 2008

If you ask an engineer whether it's safe to cross a bridge, he'll happily walk you through how safe bridges are, how the mathematics work out, how far we've come in structural safety. You'll come away from the conversation feeling confident that no bridge will ever collapse anywhere on the face of the Earth. If you ask a software engineer about banks, however, you'll likely come away terrified, with a 50/50 chance you're now convinced to put all your money in bitcoin. Banks are notorious for bad software decisions—not so much because the decisions are worse, but because most people assume banks are more careful and security-minded.

Today's submitter, Kato, worked at Inibank, where they used a commercial product called T24 as the core of their banking system. T24 is used by hundreds of banks worldwide. It's customizable for a wide range of banking solutions, and so like most large customizable suites, there are programmers that specialize in writing custom code for it and consultants that will hold your hand through major upgrades.

Inibank brought in a consultant to take on a special project while their resources were busy. At the end of the business day, there is a Close Of Business process that has to be done to ensure all the money gets where it's going, all the appropriate outputs are recalculated, and all the relevant reports are run. In banks, this also changes the system's date to the next working day—which is why if you do online banking on a Sunday, none of it begins to process until Monday morning. The consultant was meant to create a new report that would run during the Close Of Business process, which would do extra processing if it were also the End of Month.

Kato sat down with the new guy, showing him how they'd set up their end of day reports. "You see here, there's a global for the last working day, one for today, and one for the next working day. Those are YYMMDD, so they're easier to work with."

"Right, right okay, gotcha. Gotcha. And what format is that?"

"... I don't know the standard off the top of my head, but it's year, month, day."

"Right, right, okay. Cool. I'll get right to work, then."

Kato walked away from the conversation with a sinking feeling in the pit of his stomach, but he tried to ignore it. The consultant said he was all set up and ready. Surely he knew what he was doing, right? Kato put it out of his mind, and didn't worry about it again until it came time to review the code, and he found this gem:

TH.DATE = R.DATES(EB.DAT.NEXT.WORKING.DAY)[1,6]:"01"
CALL CDT('ES00',TH.DATE,"-1C")
WTODAY = OCONV(DATE(),"DY") : FMT(OCONV(DATE(),"DM"),'R%2') : FMT(OCONV(DATE(),"DD"),'R%2')
IF TH.DATE EQ WTODAY THEN

In layman's terms:

  1. Take the next working day and change the day to 01 in order to get the first day of the month.
  2. Change that date by subtracting 1 calendar day, in the Spain calendar.
  3. Take the server's date and put in format YYYYMMDD by calling 3 times for the command Date.
  4. If the date calculated in step 2 is the same as the date calculated in step 3, run the process.

Now, this ... works. Mostly. Except that if for some reason close of day ticked over past midnight on the second-to-last day of the month—not uncommon—it would incorrectly think it was the last day of the month and run the report. Which worked well with the next problem: if the same thing happened on the last day of the month, it would incorrectly not run the report. And the best bug yet: if the last day of the month was a Sunday, the server's calendar would never be set to it, since it skips non-working days.

Speaking of non-working days: as Inibank was US-based, there was no reason to use the Spain calendar. Sure, your months and weeks are the same, but the US bank holidays would need to be set in the Spain calendar in the software, or else it would still run. Finally, as if all this weren't bad enough, calling for the Date three times meant you could have inconsistencies if it straddled midnight exactly: the month retrieved before midnight, the day after.

Kato put in a comment, suggesting that the code be changed to:

IF R.DATES(EB.DAT.TODAY)[5,2] # R.DATES(EB.DAT.LAST.WORKING.DAY)[5,2] THEN

Five minutes later, the contractor was at his desk. "What is this change?"

Kato was in no mood to argue by this point. "Your code's broken, dude. You didn't need to do all that."

"I see, I see. Well, it's just, this is the standard operating procedure in the industry. But no matter."

Kato highly doubted this, but he shrugged it off. "Industry's wrong, then. I explained it all in the comment."

"I see. Yes, I read this. But I will read it again." And then he was gone, as suddenly as he'd appeared at Kato's desk.

The edits were made, Kato approved the PR, and the consultant vanished into the night.

Sometimes, late at night, Kato lies awake wondering if the consultant really understood what he did wrong, or if he were just paying lip service, collecting his check, and writing terrible code for twice Kato's salary somewhere else. Somewhere without in-house staff to look over the code.

But don't put all your money in bitcoin. That's even worse.

[Advertisement] Forget logs. Next time you're struggling to replicate error, crash and performance issues in your apps - Think Raygun! Installs in minutes. Learn more.


Error'd: Killer Errors;

Adam wrote, "I hear that NewFeature1 is a real, ahem, killer feature of these Wi-Fi drivers."

 

"Gigabyte Easy Tune6 installer must release to the failure! There is no other option!" writes Jeff B.

 

"DPD is having a really hard time guessing which month my Ebay delivery will take place," Rupert W. writes.

 

"So this USB expander can power all my peripherals, an old timey typewriter, AND a fan? Sold."

 

"My wife is flying from Chicago to Atlanta and had to reschedule," Casey B. wrote, "Luckily, she'll arrive almost two days before she leaves."

 

Chris D. writes, "So is this like malware credit? Can I run a couple of thousand malicious programs before I need to worry about infection?"

 

[Advertisement] ProGet supports your applications, Docker containers, and third-party packages, allowing you to enforce quality standards across all components. Download and see how!


CodeSOD: Swing and You're Out;

George G was hired to do some UI work for a company which sold a suite of networking hardware. As networking hardware needs to be highly configurable, George was hired on to do “some minor tweaks” to the UI. “It’s just some sizing issues, fonts, the like. I’m sure you’ll do something to the stylesheets or whatever,” said the boss.

The boss didn’t know what they were talking about. The UI for some of the products was a web based config tool. That was their consumer-grade products. Their professional grade products used the same Java program which was originally released 15 years earlier. There were no stylesheets. Instead, there was an ancient and wobbling pile of Java Swing UI code, maintained by a “master” who had Strong Opinions™ about how that code should look.

For example, dispatching a call to a method is indirection. Indirection is confusing. So inline those calls, especially if there's a conditional involved: inline all the “ifs”. Factory methods and other tools for constructing complex objects are confusing, so always inline your calls to constructors, and always pass as many parameters as you can, except for the times where you don’t do that, because why would we be consistent about anything?

All of the developers on the project had to attend to the master’s wishes during code reviews, where the master gleefully unrefactored code “to make it more clear.”

Also, keep in mind that this UI started back in an era where “800x600” was a viable screen resolution, and in fact, that’s the resolution it was designed against. On a modern monitor, it’s painfully tiny, stuffed with tabs and icons and other UI widgets. Over the years, the UI code has been tweaked to handle edge cases: one customer had the UI zoom turned on, so now there were piles of conditionals to check if the UI needed to be re-laid out. Somebody got a HiDPI display, so again, a bunch of checks and custom code paths, all piled together.

Speaking of layout, Swing was a prime case of Java taking object orientation to the extreme, so in addition to passing in widgets you want displayed, you also can supply a layout object which decides how to fill the frame. There was a whole library of them, but if you wanted a flexible layout that also handled screen scaling well, you had to use the most complicated one: Grid Bag. The Grid Bag, as the name implies, is a grid, but where the grid cells can be arbitrary sizes. You control this by adding constraints to the flow. It’s peak Java overcomplification, so even simple UIs tend to get convoluted, and with the “inline all the things” logic, you end up with code like this:

if( os.getSystemFontSize( NORMAL ) == 14 )
text = new JText("5", new GridBagConstraints(3, 1, 3,3, 1.0, 0.5, GridBagConstraints.PAGE_END, 1.5, Insets(10,5,10,5, 5, 5 ) );
else   
text = new JText("5", new GridBagConstraints(3, 1, 3,3, 0.99, 0.4, GridBagConstraints.PAGE_END, 1.5, Insets(4,2,4,5, 5, 5  ) );

This particular code checks to see if the user has their font set to 14pt. If they do, we’ll set a constraint one way. If it’s any other value, we’ll set the constraint a different way. What is the expected result of that constraint? Why all this just to display the number 5? There are a lot of numbers other than 14, and they’re all going to impact the layout of the screen.

George made it two months, and then quit. This happened just a week after another developer had quit. Another quit a week later. No one in the management chain could understand why they were losing developers so quickly, with only the master remaining behind.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!


CodeSOD: Remove This;

Denae inherited some 90s-era GUI code. The original developers have long since gone away, and the source control history has vanished into oblivion. In the 90s, the Standard Template Library for C++ was still very new, so when Denae needs to debug things in this application, it means doing some code archaeology and picking through home-brewed implementations of common data structures.

Denae’s most recent bug involved understanding why some UI elements weren’t updating properly. The basic logic the application used is that it maintained a List of GUI items which need to be repainted. So Denae dug into the the List implementation.

template <class Type> void List<Type>::Remove(Type t)
{
	int i;
	for (i=(num-1); i>=0; i--)
	{
		if(element[i] == t)
		{
			break;
		}
	}
	LoudAssert(i>=0);
	DelIndex(i);
}

template <class Type> void List<Type>::DelIndex(int i)
{
	LoudAssert(i<num);
	num--;
	while(i<num)
	{
		element[i] = element[i+1];
		i++;
	}
}

Let’s start by talking about LoudAssert. Denae didn’t provide the implementation, but had this to say:

LoudAssert is an internally defined assert that is really just an fprintf to stderr. In our system stderr is silenced, always, so these asserts do nothing.

LoudAssert isn’t an assert, in any meaningful way. It’s a logging method which also doesn’t log in production. Which means there’s nothing that stops the Remove method from getting a negative index to remove- since it loops backwards- and passing it over to DelIndex. If you try and remove an item which isn’t in the list, that’s exactly what happens. And note how num, the number of items in the list, gets decremented anyway.

Denae noticed that this must be the source of the misbehaving UI updates when the debugger told her that the list of items contained -8 entries. She adds:

We have no idea how this ever worked, or what might be affected by fixing it, but it’s been running this way for over 20 years

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!


Exceptionally Serial;

You may remember Kara, who recently found some "interesting" serialization code. Now, this code happens to be responsible for sending commands to pieces of machine equipment.

Low-level machine interfaces remain one of the domains where serial protocols rule. Serial communications use simple hardware and have minimal overhead, and something like RS232 has been in real-world use since the 60s. Sure, it's slow, sure it's not great with coping with noise, sure you have to jump through some hoops if you want a connection longer than 15m, but its failures are well understood.

Nothing is so well understood that some developer can't make a mess of it.

Public Function SendCommand(ByVal cmd As String) As String Dim status As Integer ' Write cmd to the serial port using a protocol that is too painful to reproduce here. ' status receives an appropriate value along the way as the protocol checks for various error ' conditions including timeout If status <> 0 Then Throw MakeComPortException(status) End Function Private Function MakeComPortException(ByVal status As Integer) As ComPortException Dim code As Integer Dim message As String = Nothing GetErrorCode(status, code, message) Return New ComPortException(code, message) End Function Private Sub GetErrorCode(ByVal ErrorNum As Integer, ByRef code As Integer, ByRef message As String) code = ErrorNum Select Case ErrorNum Case 129 : message = "Hardware error occured during Send Data" ' Talk Error' Case 130 : message = "System asked to talk but did not recieve Previous Talk Command" ' Nothing to say Case 131 : SendCommand("ERRMS?") Dim EMsg As String = GetResponse() Dim EmsgStart As Integer = EMsg.IndexOf(" (") Try If EMsg.Contains("ERR=") Then code = CInt(EMsg.Substring(4, EmsgStart - 4)) Catch ex As Exception End Try message = EMsg.Substring(EmsgStart) Case 132 : message = "H/W Error while system trying to accept data" 'Listen Error Case 133 : message = "More than 80 characters received before term char" Case 134 : message = "Archive media is full" Case 135 : message = "Listen state interrupted by ESC key" ' Interrupted from keyboard Case 136 : message = "Listen state interrupted by controller sending '*'" ' Interrupted by Controller Case 137 : message = "Error Occured in UART" Case StatusCodes.PortDeviceNotFoundErrorCode : message = "No device Found" Case StatusCodes.PortTimeoutErrorCode : message = "COM port Timeout Error" ' This next occurs if cable is unplugged at controller Case StatusCodes.PortDisconnectedErrorCode : message = "Serial cable disconnected" Case Else : message = "Error #: " & ErrorNum End Select End Sub

So, the SendCommand method takes a string and passes it down the serial port. The protocol details were elided here, but we know that we receive a status number. MakeComPortException takes that number and helpfully looks up the message which goes with it, using GetErrorCode.

GetErrorCode is one gigantic switch statement. And let's pay close attention to the message lookup process for error 131. You'll note that we call SendCommand to ask the remote device to tell us what the error message was. But in some cases, it's going to reply to that request with an error code. Error 131, to be exact.

So if we trace this: we call SendCommand which gets a 131 error, which forces it to throw the results of calling MakeComPortException, which calls GetErrorCode, which calls SendCommand, which throws a new MakeComPortException, which…

There's an interesting side effect of this approach. Despite looking like a series of recursive calls, throw unwinds the stack, so this code will never actually trigger a stack overflow. It's actually more of an exception-assisted infinite loop.

For a bonus, note the PortTimeoutErrorCode entry. On the hardware side, they use a custom serial cable which wires a loopback on the RS232 "Ready to Send" and "Clear to Send" pins, which the software uses to detect that the cable is unplugged. It also has the side effect of ensuring that no off-the-shelf RS232 cables will work with the software. This is either a stupid mistake, or a fiendishly clever way to sell heavily marked-up replacement cables.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!


Portage and Portability;

ST 225 20MB drive and WDC controller

Many moons ago, when PCs came housed within heavy cases of metal and plastic, Matt Q. and his colleague were assigned to evaluate a software package for an upcoming sales venture. Unfortunately, he and the colleague worked in different offices within the same metro area. As this was an age bereft of effective online collaboration tools, Matt had to travel regularly to the other office, carrying his PC with him. Each time, that meant unscrewing and unhooking the customary 473 peripheral cables from the back of the box, schlepping it through the halls and down the stairs, and catching the bus to reach the other office, where he got to do all those things again in reverse order. When poor scheduling forced the pair to work on the weekend, they hauled their work boxes between apartments as well.

As their work proceeded, Matt reached the limits of what his 20 MB hard drive could offer. From his home office, Matt filed a support ticket with IT. The technician assigned to his ticket—Gary—arrived at Matt's cubicle some time later, brandishing a new hard drive and a screwdriver. Gary shooed Matt away for more coffee to better focus on his patient. One minor surgery later, Matt's PC was back up and running with a bigger hard drive.

One day ahead of the project deadline, Matt was nearly done with his share of the work. He just had a few tweaks to make to his reports before copying them to the floppy disks needed by the sales team. Having hooked his PC back up within his cubicle, he switched it on—only to be greeted with a literal bang. The PC was dead and would not start.

After a panicked call to IT, Gary eventually reappeared at his desk with a screwdriver. Upon cracking open the PC case, he immediately cried, "Wait a minute! Have you been carting this PC around?"

Matt frowned. "Er, yes. Is that a problem?"

"I'll say! You weren't supposed to do that!" Gary scolded. "The hard drive's come loose and shorted out the workings!"

Matt darted over to Gary's side so he could see the computer's innards for himself. It didn't take long at all to notice that the new hard drive had been "secured" into place using Scotch tape.

"Hang on! I daresay you weren't supposed to do that!" Matt pointed to the offending tape. "Shall I check with your manager to be on the safe side?"

Gary's face crumpled. "I don't have access to the proper mountings!"

"Then find someone who does!"

Armed with his looming deadline and boss' approval, Matt escalated his support ticket even higher. It didn't take long at all for genuine mounting brackets to replace the tape. He never learned why IT techs were being deprived of necessary hardware; he assumed it was some fool's idea of a brilliant cost-cutting measure. He had to wonder how many desperate improvisations held their IT infrastructure together, and how much longer they would've gone unnoticed if it hadn't been for his PC-toting ways.

[Advertisement] Forget logs. Next time you're struggling to replicate error, crash and performance issues in your apps - Think Raygun! Installs in minutes. Learn more.