cal

Posted by Lyle Seaman

https://thedailywtf.com/articles/three-blinded-mice

...sent us five wtfs. And so on anon.

Item the first, an anon is "definitely not qualified" for this job. "These years of experience requirements are getting ridiculous."

Item the second unearthed by a farmanon has a loco logo. "After reading about the high quality spam emails which are indistinguishable from the company's emails, I got one from the spammer just starting his first day."

In thrid place, anon has only good things to say: "I'm liking their newsletter recommendations so far."

"A choice so noice, they gave it twoice," quipped somebody.

And foinally, a tdwtfer asks "I've seen this mixmastered calendering on several web sites. Is there an OSS package that is doing this? Or is it a Wordpress plugin?" I have a sneaking suspicion I posted this before. Call me on it.

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

https://thedailywtf.com/articles/three-blinded-mice

Posted by Remy Porter

https://thedailywtf.com/articles/terned-backwards

Antonio has an acquaintance has been seeking career advancement by proactively hunting down and fixing bugs. For example, in one project they were working on, there was a bug where it would incorrectly use MiB for storage sizes instead of MB, and vice-versa.

We can set aside conspiracy theories about HDD and RAM manufacturers lying to us about sizes by using MiB in marketing. It isn't relevant, and besides, its not like anyone can afford RAM anymore, with crazy datacenter buildouts. Regardless, which size to use, the base 1024 or base 1000, was configurable by the user, so obviously there was a bug handling that flag. Said acquaintance dug through, and found this:

const baseValue = useSI ? 1000 : 1024;

I know I have a "reputation" when it comes to hating ternaries, but this is a perfectly fine block of code. It is also correct: if you're using SI notation, you should do base 1000.

Now, given that this code is correct, you or I might say, "Well, I guess that isn't the bug, it must be somewhere else." Not this intrepid developer, who decided that they could fix it.

//            const baseValue = useSI ? 1000 : 1024;
            baseValue = 1024
            if (useSI === false)
            {
                baseValue = 1000;
            }
            if (useSI === true)
            {
                baseValue = 1024;
            }

It's rather amazing to see a single, correct line, replaced with ten incorrect lines, and I'm counting commenting out the correct line as one of them.

First, this doesn't correctly declare baseValue, which JavaScript is pretty forgiving about, but it also discards constness. Of course, you have to discard constness now that you've gotten rid of the ternary.

Then, our if statement compares a boolean value against a boolean literal, instead of simply if(!useSI). We don't use an else, despite an else being absolutely correct. Or actually, since we defaulted baseValue, we don't even need an else!

But of course, all of that is just glitter on a child's hand-made holiday card. The glue holding it all together is that this code just flips the logic. If we're not using SI, we set baseValue to 1000, and if we are using SI, we set it to 1024. This is wrong. This is the opposite of what the code says we should do, what words mean, and how units work.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

https://thedailywtf.com/articles/terned-backwards

Posted by Remy Porter

https://thedailywtf.com/articles/waiting-for-october

Arguably, the worst moment for date times was the shift from Julian to Gregorian calendars. The upgrade took a long time, too, as some countries were using the Julian calendar over 300 years from the official changeover, famously featured in the likely aprochryphal story about Russia arriving late for the Olympics.

At least that change didn't involve adding any extra months, unlike some of the Julian reforms, which involved adding multiple "intercalary months" to get the year back in sync after missing a pile of leap years.

Speaking of adding months, Will J sends us this "calendar" enum:

enum Calendar
{
    April = 0,
    August = 1,
    December = 2,
    February = 3,
    Friday = 4,
    January = 5,
    July = 6,
    June = 7,
    March = 8,
    May = 9,
    Monday = 10,
    November = 11,
    October = 12,
    PublicHoliday = 13,
    Saturday = 14,
    Sunday = 15,
    September = 16,
    Thursday = 17,
    Tuesday = 18,
    Wednesday = 19
}

Honestly, the weather in PublicHoliday is usually a bit too cold for my tastes. A little later into the spring, like Saturday, is usually a nicer month.

Will offers the hypothesis that some clever developer was trying to optimize compile times: obviously, emitting code for one enum has to be more efficient than emitting code for many enums. I think it more likely that someone just wanted to shove all the calendar stuff into one bucket.

Will further adds:

One of my colleagues points out that the only thing wrong with this enum is that September should be before Sunday.

Yes, arguably, since this enum clearly was meant to be sorted in alphabetical order, but that raises the question of: should it really?

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

https://thedailywtf.com/articles/waiting-for-october

Posted by Remy Porter

https://thedailywtf.com/articles/c-0-25

A good C programmer can write C in any language, especially C++. A bad C programmer can do the same, and a bad C programmer will do all sorts of terrifying things in the process.

Gaetan works with a terrible C programmer.

Let's say, for example, you wanted to see if an index existed in an array, and return its value- or return a sentinel value. What you definitely shouldn't do is this:

    double Module::GetModuleOutput(int numero) {
        double MAX = 1e+255 ;
        if (this->s.sorties+numero )
            return this->s.sorties[numero];
        else
            return MAX ;
    }

sorties is an array. In C, you may frequently do some pointer arithmetic operations, which is why sorties+numero is a valid operation. If we want to be pedantic, *(my_array+my_index) is the same thing as my_array[my_index]. Which, it's worth noting, both of those operations de-reference an array, which means you better hope that you haven't read off the end of the array.

Which is what I suspect their if statement is trying to check against. They're ensuring that this->s.sorties+numero is not a non-zero/false value. Which, if s.sorties is uninitialized and numero is zero, that check will work. Otherwise, that check is useless and does nothing to ensure you haven't read off the end of the array.

Which, Gaetan confirms. This code works "because in practice, GetModuleOutput is called for numero == 0 first. It never de-references off the end of the array, not because of defensive programming, but because it just never comes up in actual execution.

Regardless, if everything is null, we return 1e+255, which is not a meaningful value, and should be treated like a sentinel for "no real value". None of the calling code does that, however, but also, it turns out not to matter.

This pattern is used everywhere there is arrays, except the handful of places where this pattern is not used.

Then there's this one:

    if(nb_type_intervalle<1)    { }
    else 
        if((tab_intervalle=(double*)malloc(nb_lignes_trouvees*nb_type_intervalle*2 \
                                                        *sizeof(double)))==NULL)
            return(ERREUR_MALLOC);

First, I can't say I love the condition here. It's confusing to have an empty if clause. if (nb_type_intervalle>=1) strikes me as more readable.

But readability is boring. If we're in the else clause, we attempt a malloc. While using malloc in C++ isn't automatically wrong, it probably is. C++ has its own allocation methods that are better at handling things like sizes of datatypes. This code allocates memory for a large pile of doubles, and stores a pointer to that memory in tab_intervalle. We do all this inside of an if statement, so we can then check that the resulting pointer is not NULL; if it is, the malloc failed and we return an error code.

The most frustrating thing about this code is that it works. It's not going to blow up in surprising ways. I never love doing the "assignment and check" all in one statement, but I've seen it enough times that I'd have to admit it's idiomatic- to C style programming. But that bit of code golf coupled with the pointlessly inverted condition that puts our main logic in the else just grates against me.

Again, that pattern of the inverted conditional and the assignment and check is used everywhere in the code.

Gaetan leaves us with the following:

Not a world-class WTF. The code works, but is a pain in the ass to inspect and document

In some ways, that's the worst situation to be in: it's not bad enough to require real work to fix it, but it's bad enough to be frustrating at every turn.

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

https://thedailywtf.com/articles/c-0-25

Posted by Lyle Seaman

https://thedailywtf.com/articles/cruel-brittanica

"No browser is the best browser," opines Michael R. sarcastically as per usual for tdwtf. "Thank you for suggesting a browser. FWIW: neither latest Chrome, Safari, Firefox, Opera work. Maybe I should undust my Netscape."

An anonymous dessert lover ruminates "The icing on the cake is that it's for HR where names can be quite important. Just goes to show that not even SAP can do SAP."

Another anonymous dessert lover (because honestly, who isn't) cheers "2024 is back again."

Thrice capitalled B.J.H. capitulates. "I guess I'm not cleared to know what topic I subscribed to."

Jeopardy fan Jeremy P. digs a quick quiz.

It's from Britannica.com. I thought "TV remote control" because it would effectively turn off the TV. The correct answer is toaster.

 To understand what went wrong, the previous correct answer was "blunderbuss".

For a bonus buzz, Jeremy sent in another.

This time it's "guess the novel from the picture". There was a subtle clue in this one.

You're a monster, Jeremy. 

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

https://thedailywtf.com/articles/cruel-brittanica

Posted by Remy Porter

https://thedailywtf.com/articles/one-version-of-events

Jon supports some software that's been around long enough that the first versions of the software ran on, and I quote, "homegrown OS". They've long since migrated to Linux, and in the process much of their software remained the same. Many of the libraries that make up their application haven't been touched in decades. Because of this, they don't really think too much about how they version libraries; when they deploy they always deploy the file as mylib.so.1.0. Their RPM post-install scriptlet does an ldconfig after each deployment to get the symlinks updated.

For those not deep into Linux library management, a brief translation: shared libraries in Linux are .so files. ldconfig is a library manager, which finds the "correct" versions of the libraries you have installed and creates symbolic links to standard locations, so that applications which depend on those libraries can load them.

In any case, Jon's team's solution worked until it didn't. They deployed a new version of the software, yum reported success, but the associated services refused to start. This was bad, because this happened in production. It didn't happen in test. They couldn't replicate it anywhere else, actually. So they built a new version of one of the impacted libraries, one with debug symbols enabled, and copied that over. They manually updated the symlinks, instead of using ldconfig, and launched the service.

The good news: it worked.

The bad news: it worked, but the only difference was that the library was built with debug symbols. The functionality was exactly the same.

Well, that was the only difference other than the symlink.

Fortunately, a "before" listing of the library files was captured before the debug version was installed, a standard practice by their site-reliability-engineers. They do this any time they try and debug in production, so that they can quickly revert to the previous state. And in this previous version, someone noticed that mylib.so was a symlink pointing to mylib.so.1.0.bkup_20190221.

Once again, creating a backup file is a standard practice for their SREs. Apparently, way back in 2019 someone was doing some debugging. They backed up the original library file, but never deleted the backup. And for some reason, ldconfig had been choosing the backup file when scanning for the "correct" version of libraries. Why?

Here, Jon does a lot of research for us. It turns out, if you start with the man pages, you don't get a answer- but you do get a warning:

ldconfig will look only at files that are named lib*.so* (for regular shared objects) or ld-.so (for the dynamic loader itself). Other files will be ignored. Also, ldconfig expects a certain pat‐
tern to how the symbolic links are set up, like this example, where the middle file (libfoo.so.1 here) is the SONAME for the library:

libfoo.so -> libfoo.so.1 -> libfoo.so.1.12

Failure to follow this pattern may result in compatibility issues after an upgrade.

Well, they followed the pattern, and they found compatibility issues. But what exactly is going on here? Jon did the work of digging straight into the ldconfig source to find out the root cause.

The version detecting algorithm starts by looking directly at filenames. While the man page warns about a convention, ldconfig doesn't validate names against this convention (which is probably the correct decision). Insetad, to find which filename has the highest version number, it scans through two filenames until finds numeric values in both of them, then does some pretty manual numeric parsing:

int _dl_cache_libcmp(const char *p1, const char *p2) {
  while (*p1 != '\0') {
    if (*p1 >= '0' && *p1 <= '9') {
      if (*p2 >= '0' && *p2 <= '9') {
        /* Must compare this numerically.  */
        int val1;
        int val2;

        val1 = *p1++ - '0';
        val2 = *p2++ - '0';
        while (*p1 >= '0' && *p1 <= '9')
          val1 = val1 * 10 + *p1++ - '0';
        while (*p2 >= '0' && *p2 <= '9')
          val2 = val2 * 10 + *p2++ - '0';
        if (val1 != val2)
          return val1 - val2;
      } else
        return 1;
    } else if (*p2 >= '0' && *p2 <= '9')
      return -1;
    else if (*p1 != *p2)
      return *p1 - *p2;
    else {
      ++p1;
      ++p2;
    }
  }
  return *p1 - *p2;
}

NB: this is the version of ldconfig at the time Jon submitted this, and the version that they're using. I haven't dug through to check if this is still true in the latest version. That's an exercise for the reader.

While we have not hit the end of the first string, check if the character in that string is numeric. If it is, check if the character in the second string is numeric. If it is, keep scanning through characters, and for as long as they're numeric, keep parsing them into numbers. If the numbers aren't the same, we return the difference between them.

If the first string contains numbers at this point, but the second string doesn't, return 1. If the second string contains numbers but not the first, return -1. Otherwise, increment our pointers and go to the next character. If we reach the end of the string without finding numeric characters, return the difference between these two characters.

Also, correct me if I'm wrong, but it seems like a malicious set of filenames could cause buffer overruns here.

Now, I'll be honest, I don't have the fortitude to suggest that ldconfig is TRWTF here. It's a venerable piece of software that's solving an extremely hard problem. But boy, DLL Hell is an unending struggle and this particular solution certainly isn't helping. I'm honestly not entirely certain I'd say that there was a true WTF here, just an unfortunate confluence of people doing their best and ending up laying landmines for others.

But here's the fun conclusion: the 2019 version of the library actually had been updated. They'd deployed several new versions between 2019 and 2024, when things finally blew up. The actual deployed software kept using the backup file from 2019, and while it may have caused hard-to-notice and harder-to-diagnose bugs, it didn't cause any crashes until 2024.

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

https://thedailywtf.com/articles/one-version-of-events

Posted by Remy Porter

https://thedailywtf.com/articles/invalid-passport

Gretchen wanted to, in development, disable password authentication. Just for a minute, while she was testing things. That's when she found this approach to handling authentication.

passport.authenticate('local', { session: true }, async (err, user) => {
  if (err) {
    res.send({ success: false, message: 'Error authenticating user.' })
  } else if (!user) {
    User.query()
      .where({ username: req.body.username })
      .first()
      .then(targetUser => {
        if (targetUser) {
          const hash = User.hashPassword(
            targetUser.password_salt,
            req.body.password
          )
          if (hash === targetUser.password_hash) {
            res.send({
              success: false,
              message: 'Incorrect username or password.',
            })
          } else {
            res.send({
              success: false,
              message: 'Incorrect username or password.',
            })
          }
        } else {
          res.send({
            success: false,
            message: 'Incorrect username or password.',
          })
        }
      })
      .catch(err => {
        res.send({ success: false, message: 'Internal server error' })
      })
  } else if (user.firstLogin) {
//......
  }
})(req, res, next);

passport.authenticate invokes its callback after attempting to authenticate. Now, normally, this is called as middleware on a route defined on the webserver- that is to say, you don't call it from within your application code, but as part of your routing configuration. That's not the case here, where this blob is inside of a controller.

That's weird, but let's just trace through this code. We attempt to authenticate. When the process completes, our callback function executes. If authentication failed, there's an error, so we'll send an error message. Then, if the user object isn't populated, we attempt to look up the user. If we find a user with that user name, we then hash their password and check if the hash matches. If it does, we send an error message. If it doesn't, we send an error message. If we didn't find the user, we send an error message. If anything goes wrong, we send an error message.

Wait a second, back up: if the user exists and their password matches, we send an error message?

I'll let Gretchen explain a bit more:

passport.authenticate returns an error if the authentication failed and a user object, if it succeeded. We check this immediately: if error is set, return an error message. But then, we check if the user does not exist (aka: the authentication failed).

Yes, the reason user would be null is because the authentication failed. So the error would be set. So that entire branch about validating the user won't happen: either the authentication worked and we know who the user is, or it failed, in which case we'd have an error. There's no reasonable world where there's no error but also no user object.

So yes, if authentication failed, but you manually re-run the authentication and it succeeds for some reason, yeah, you probably should still return an error. But I don't know if it's "Incorrect username or password". It probably should be "Invalid reality, please restart the universe and see if the problem persists."

https://thedailywtf.com/articles/invalid-passport