Computer
CodeSOD: IsValidToken
To ensure that several services could only be invoked by trusted parties, someone at Ricardo P's employer had the brilliant idea of requiring a token along with each request. Before servicing a request, they added this check:
private bool IsValidToken(string? token) { if (string.Equals("xxxxxxxx-xxxxxx+xxxxxxx+xxxxxx-xxxxxx-xxxxxx+xxxxx", token)) return true; return false; }The token is anonymized here, but it's hard-coded into the code, because checking security tokens into source control, and having tokens that never expire has never caused anyone any trouble.
Which, in the company's defense, they did want the token to expire. The problem there is that they wanted to be able to roll out the new token to all of their services over time, which meant the system had to be able to support both the old and new token for a period of time. And you know exactly how they handled that.
private bool IsValidToken(string? token) { if (string.Equals("xxxxxxxx-xxxxxx+xxxxxxx+xxxxxx-xxxxxx-xxxxxx+xxxxx", token)) return true; else if (string.Equals("yyyyyyy-yyyyyy+yyyyy+yyyyy-yyyyy-yyyyy+yyyy", token)) return true; return false; }For a change, I'm more mad about this insecurity than the if(cond) return true pattern, but boy, I hate that pattern.
[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!Visa and Mastercard Are Getting Overwhelmed By Gamer Fury Over Censorship
Read more of this story at Slashdot.
Claude Code Users Hit With Weekly Rate Limits
Read more of this story at Slashdot.
Bankrupt Futurehome Suddenly Makes Its Smart Home Hub a Subscription Service
Read more of this story at Slashdot.
A Second Tea Breach Reveals Users' DMs About Abortions and Cheating
Read more of this story at Slashdot.
Anker Is No Longer Selling 3D Printers
Read more of this story at Slashdot.
OpenAI's ChatGPT Agent Casually Clicks Through 'I Am Not a Robot' Verification Test
Read more of this story at Slashdot.
Say Goodbye To Your Custom ROMs As Samsung's One UI 8 Kills Bootloader Unlock
Read more of this story at Slashdot.
Cyberattack Cripples Russian Airline Aeroflot
Read more of this story at Slashdot.
Tesla Signs $16.5 Billion Contract With Samsung To Make AI Chips
Read more of this story at Slashdot.
Microsoft Adds Copilot Mode To Edge
Read more of this story at Slashdot.
Chinese Universities Want Students To Use More AI, Not Less
Read more of this story at Slashdot.
Nearly Half of US Venture Capital Professionals in Middle To Senior Positions Have No Successful Investments
Read more of this story at Slashdot.
Windows 11 is a 'Minefield of Micro-aggressions in the Shipping Lane of Progress'
Read more of this story at Slashdot.
Security Researchers Find Evidence SkyRover X1 Is Disguised DJI Product
Read more of this story at Slashdot.
Can a Country Be Too Rich? Norway Is Finding Out
Read more of this story at Slashdot.
Ageing Accelerates at Around Age 50 - Some Organs Faster Than Others
Read more of this story at Slashdot.
Google's New Security Project 'OSS Rebuild' Tackles Package Supply Chain Verification
Read more of this story at Slashdot.
Astronomers Use Black Holes to Pinpoint Earth's Location. But are Phones and Wifi Blocking the View?
Read more of this story at Slashdot.
CodeSOD: An Exert Operation
The Standard Template Library for C++ is… interesting. A generic set of data structures and algorithms was a pretty potent idea. In practice, early implementations left a lot to be desired. Because the STL is a core part of C++ at this point, and widely used, it also means that it's slow to change, and each change needs to go through a long approval process.
Which is why the STL didn't have a ``std::map::containsfunction until the C++20 standard. There were other options. For example, one could usestd::map::count, to count how many times a key appear. Or you could use std::map::findto search for a key. One argument against adding astd::map::containsfunction is thatstd::map::count` basically does the same job and has the same performance.
None of this stopped people from adding their own. Which brings us to Gaetan's submission. Absent a std::map::contains method, someone wrote a whole slew of fieldExists methods, where field is one of many possible keys they might expect in the map.
bool DataManager::thingyExists (string name) { THINGY* l_pTHINGY = (*m_pTHINGY)[name]; if(l_pTHINGY == NULL) { m_pTHINGY->erase(name); return false; } else { return true; } return false; }I've head of upsert operations- an update and insert as the same operation, but this is the first exert- an existence check and an insert in the same operation.
"thingy" here is anonymization. The DataManager contained several of these methods, which did the same thing, but checked a different member variable. Other classes, similar to DataManager had their own implementations. In truth, the original developer did a lot of "it's a class, but everything inside of it is stored in a map, that's more flexible!"
In any case, this code starts by using the [] accessor on a member variable m_pTHINGY. This operator returns a reference to what's stored at that key, or if the key doesn't exist inserts a default-constructed instance of whatever the map contains.
What the map contains, in this case, is a pointer to a THINGY, so the default construction of a pointer would be null- and that's what they check. If the value is null, then we erase the key we just inserted and return false. Otherwise, we return true. Otherotherwise, we return false.
As a fun bonus, if someone intentionally stored a null in the map, this will think the key doesn't exist and as a side effect, remove it.
Gaetan writes:
What bugs me most is the final, useless return.
I'll be honest, what bugs me most is the Hungarian notation on local variables. But I'm long established as a Hungarian notation hater.
This code at least works, which compared to some bad C++, puts it on a pretty high level of quality. And it even has some upshots, according to Gaetan:
On the bright side: I have obtained easy performance boosts by performing that kind of cleanup lately in that particular codebase.
[Advertisement] Picking up NuGet is easy. Getting good at it takes time. Download our guide to learn the best practice of NuGet for the Enterprise.