'I bet Russian hackers weren't expecting their target to suck so epically hard as this'

It's all fun and games until someone gets hurt


Line Break Welcome back to Line Break, our weekly column of terrible code our readers have encountered in the wild.

So far we've featured astonishingly brain-dead designs in production and amusing code from yesteryear machines. Our emphasis has been on learning through others' mistakes while also brightening drab Wednesday mornings with a chuckle and sage shake of the head.

It's all good fun until someone gets hacked.

SQL injections are still real in 2016

Reg reader Daniel is seething: the buggy code below was almost exploited by Russian hackers, we're told.

"This morning I encountered a classic SQL injection vulnerability which I recount here to demonstrate that really, really bad programmers still abound and are not restricted to ancient FORTRAN or COBOL," said Daniel, who described himself as livid.

The vulnerable code was imported from an external team, we're told: "The villain in this piece is a professional company, or I could say cowboy posse, that should know better and is big enough to afford better. If PHP has a bad reputation, it is because of imbeciles like these.

"Our site was attacked as a result of the following..."

$path = (string) getURLParam('path');
if($path) {
  $path[0] == "\/" ? $path = substr($path, 1, strlen($path)) : $path;

  $tableName = getTableName();
  $write = getDBConnection();

  $query = "select MAIN_TABLE.`product_id` from `{$tableName}` as MAIN_TABLE where MAIN_TABLE.`request_path` in('{$path}')";
  $readresult=$write->query($query);
  if ($row = $readresult->fetch() ) {
    $productId=$row['product_id'];
  }
}

This source really sucks. It lets an attacker inject arbitrary SQL commands into the code. We've changed some of the function names so no one innocent gets in trouble. All you need to know is that $path is set directly from the HTTP request sent to the web app, and thus under attacker control, ultimately potentially leading to SQL injection. The weird use of in(), though, thwarted initial exploitation of the SQL hole because it was unexpected by the hackers. It wouldn't have taken long for the miscreants to have worked around it.

"The main problem is $path goes unquoted and so any page request with a single quote may execute any SQL statement," explained Daniel.

"The incompetence doesn't stop there, I also see: the pointless use of a ternary operator when there is no 'else' action; raw SQL in a controller; the direct use of a database resource; not checking if product_id is valid – this could easily be added to the query's where clause if they feel PHP is too difficult; and the bizarre use of in() instead of =, although this strange practice ultimately saved our site because out of the hundreds of attacks we received, none had the crucial "');" needed to end the first statement.

"I bet the Russian hackers weren't expecting their target to suck so epically hard as this!"

Lesson: Prepared SQL statements – use them. Never pass raw user input direct to sensitive code without sanitizing it. Just don't write PHP like the above. It's 2016. Daniel, you have our sympathies.

Whisky Tango Foxtrot

The above code is dangerous to system security. The following is dangerous to mental security. Rafa writes in with some more programming misery.

"I used to work with someone who wrote this," we're told.

    /**
     *
     * @param alpha first item
     * @param omega last item
     * @return true if the last item is null
     */
    public static boolean omegaIsNull(Object alpha, Object omega) {
        boolean eval = false;
        if (alpha == null) {

            eval = false;
        }

        if (alpha != null && omega == null) {

            eval = true;
        }

        if ((alpha != null & !alpha.equals("") & !alpha.equals(" ")) && (omega == null || omega.equals("") || omega.equals(" "))) {

            eval = true;
        }

        return eval;
    }

Argh. ARRGHHH! Why not just simply check for omega == null?

"Now," Rafa warns us, "take a look at the sloppy use of operators: logical (&, |) and conditional (&&, ||) are used apparently at random with disastrous results – NullPointerException for example.

"I simply cannot explain my astonishment. Why all this 27 lines, when all you need is 11 characters? Needless to say, this method does not work at all.

"Lessons learned? KISS. Reduce the code to a bare minimum. Select big blocks and press the delete key with joy. It can only work better."

Lesson: It would easy to say something like "fire them". But that doesn't help at all. Learn that more code does not equal better code. The best code gets the job done and only the job required done, no ifs and no buts. Well, maybe some if()s.

Unsign me up

Let's finish on a happy note rather than an angry one. Paul, a software engineer for 20 years, has sent us in a hat trick of cockups:

"I once worked with an engineer who had been asked to investigate why serial communications with an embedded device no longer worked in a program someone wrote years before. On investigation, it turned out that an essential delay was being done by means of a for loop (eg: for(int i=1; i <10000; i++) {}).

"As soon as computers got a little bit quicker, the delay became shorter and communications was no longer possible. You can probably guess what he did to fix it."

Lesson: We've all made a bodged-in timing loop to get characters out to a serial port or similar in our time. It worked, we saw some text, we knew the board was working. Crucially, you then make the timing code a little more sophisticated. It only takes a few extra minutes. Skip the second pint at lunch and implement it, ta.

Next.

"About 15 years ago, one of our engineers was asked to rewrite an old Delphi application in C++ as the company had decided to standardized on C based languages," Paul continued.

"At the time, this application used a proprietary binary file format, with many settings being bit encoded. To encode these bits, the engineer decided to bitwise OR the destination byte with 2 raised to a particular power representing the bit position.

"Bad approach, I know, but it was made worse by the fact that the engineer had a Visual Basic background. In Visual Basic, raising something to power of another value can be expressed as ^, eg: x = x || (2^3).

"In C based languages, ^ means bitwise XOR. This resulted in numerous customer files being saved incorrectly and requiring significant manual effort on our part to restore them. Not a good time."

Lesson: Different languages do bitwise operations differently. Also, there's got to be a better way to store configuration data than using raw binary. JSON, YAML, or (dare I say it) XML are pretty cool instead.

Next.

"Many engineers I know, me included, have fallen foul of counting backwards down to zero using an unsigned variable instead of a signed one," says Paul. "This results in a loop from which there is no escape, for example..."

for(uint i = 100; i >= 0; i--)
{
  //in here forever
}

Lesson: Unsigned integers do not cross zero to -1, they'll typically roll around to their maximum value – a very positive number. Either stop at zero or use a signed integer instead.

Phew, OK. That's enough pain. We'll see you next week for some more therapy. Send in your code submissions, please, we love to hear from you. ®

Click here to see all Line Break columns


Other stories you might like

  • Lonestar plans to put datacenters in the Moon's lava tubes
    How? Founder tells The Register 'Robots… lots of robots'

    Imagine a future where racks of computer servers hum quietly in darkness below the surface of the Moon.

    Here is where some of the most important data is stored, to be left untouched for as long as can be. The idea sounds like something from science-fiction, but one startup that recently emerged from stealth is trying to turn it into a reality. Lonestar Data Holdings has a unique mission unlike any other cloud provider: to build datacenters on the Moon backing up the world's data.

    "It's inconceivable to me that we are keeping our most precious assets, our knowledge and our data, on Earth, where we're setting off bombs and burning things," Christopher Stott, founder and CEO of Lonestar, told The Register. "We need to put our assets in place off our planet, where we can keep it safe."

    Continue reading
  • Conti: Russian-backed rulers of Costa Rican hacktocracy?
    Also, Chinese IT admin jailed for deleting database, and the NSA promises no more backdoors

    In brief The notorious Russian-aligned Conti ransomware gang has upped the ante in its attack against Costa Rica, threatening to overthrow the government if it doesn't pay a $20 million ransom. 

    Costa Rican president Rodrigo Chaves said that the country is effectively at war with the gang, who in April infiltrated the government's computer systems, gaining a foothold in 27 agencies at various government levels. The US State Department has offered a $15 million reward leading to the capture of Conti's leaders, who it said have made more than $150 million from 1,000+ victims.

    Conti claimed this week that it has insiders in the Costa Rican government, the AP reported, warning that "We are determined to overthrow the government by means of a cyber attack, we have already shown you all the strength and power, you have introduced an emergency." 

    Continue reading
  • China-linked Twisted Panda caught spying on Russian defense R&D
    Because Beijing isn't above covert ops to accomplish its five-year goals

    Chinese cyberspies targeted two Russian defense institutes and possibly another research facility in Belarus, according to Check Point Research.

    The new campaign, dubbed Twisted Panda, is part of a larger, state-sponsored espionage operation that has been ongoing for several months, if not nearly a year, according to the security shop.

    In a technical analysis, the researchers detail the various malicious stages and payloads of the campaign that used sanctions-related phishing emails to attack Russian entities, which are part of the state-owned defense conglomerate Rostec Corporation.

    Continue reading
  • FTC signals crackdown on ed-tech harvesting kid's data
    Trade watchdog, and President, reminds that COPPA can ban ya

    The US Federal Trade Commission on Thursday said it intends to take action against educational technology companies that unlawfully collect data from children using online educational services.

    In a policy statement, the agency said, "Children should not have to needlessly hand over their data and forfeit their privacy in order to do their schoolwork or participate in remote learning, especially given the wide and increasing adoption of ed tech tools."

    The agency says it will scrutinize educational service providers to ensure that they are meeting their legal obligations under COPPA, the Children's Online Privacy Protection Act.

    Continue reading
  • Mysterious firm seeks to buy majority stake in Arm China
    Chinese joint venture's ousted CEO tries to hang on - who will get control?

    The saga surrounding Arm's joint venture in China just took another intriguing turn: a mysterious firm named Lotcap Group claims it has signed a letter of intent to buy a 51 percent stake in Arm China from existing investors in the country.

    In a Chinese-language press release posted Wednesday, Lotcap said it has formed a subsidiary, Lotcap Fund, to buy a majority stake in the joint venture. However, reporting by one newspaper suggested that the investment firm still needs the approval of one significant investor to gain 51 percent control of Arm China.

    The development comes a couple of weeks after Arm China said that its former CEO, Allen Wu, was refusing once again to step down from his position, despite the company's board voting in late April to replace Wu with two co-chief executives. SoftBank Group, which owns 49 percent of the Chinese venture, has been trying to unentangle Arm China from Wu as the Japanese tech investment giant plans for an initial public offering of the British parent company.

    Continue reading
  • SmartNICs power the cloud, are enterprise datacenters next?
    High pricing, lack of software make smartNICs a tough sell, despite offload potential

    SmartNICs have the potential to accelerate enterprise workloads, but don't expect to see them bring hyperscale-class efficiency to most datacenters anytime soon, ZK Research's Zeus Kerravala told The Register.

    SmartNICs are widely deployed in cloud and hyperscale datacenters as a means to offload input/output (I/O) intensive network, security, and storage operations from the CPU, freeing it up to run revenue generating tenant workloads. Some more advanced chips even offload the hypervisor to further separate the infrastructure management layer from the rest of the server.

    Despite relative success in the cloud and a flurry of innovation from the still-limited vendor SmartNIC ecosystem, including Mellanox (Nvidia), Intel, Marvell, and Xilinx (AMD), Kerravala argues that the use cases for enterprise datacenters are unlikely to resemble those of the major hyperscalers, at least in the near term.

    Continue reading

Biting the hand that feeds IT © 1998–2022