X

Aggregator

CodeSOD: Authorized Logger

The Daily WTF
6 hours 21 minutes ago

Gretchen's company recently got purchased by Initech. Specifically, they were bought for their dev team, of all things. They had a few software products that were high performers, and Initech wanted that secret sauce. They bought the company, and then split the dev team up and migrated the developers to new products.

That actually worked out okay for Gretchen, most of the time. For a few projects, the dev team was given some requirements and a free hand to figure out how to deliver them. They were free to reuse code that existed or rewrite entirely, based on their own judgement. They were free to pick the tools they wanted to use, and the results worked out well.

But there were some projects that… were a different story. After those successes, Gretchen got moved onto a project that was 90% firefighting. The app had code like this:

req.body.externalId = !!req.body.externalId ? req.body.externalId + "" : "";

How's that for some null handling.

The whole thing can't run on a version of NodeJS newer than 14: a version that last got an update in 2023.

"The code follows no conventions," Gretchen writes, "there's no logging."

exports.create = (req, res) => { logger.debug('creating new staffClient'); logger.debug(req.body) // let staffClient = new StaffClient({}); // // run through and create all fields on the model // for(var k in req.body) { // if(req.body.hasOwnProperty(k)) { // staffClient[k] = req.body[k]; // } // } StaffClient.query().insert(req.body) .returning('*') .then(staffClient => { if(staffClient) { res.send({success: true, staffClient}) } else { res.send({ success: false, message: "Could not save StaffClient"}) } }); }

Now, you may say to yourself, "What do you mean there's no logging? I see it right there!" There is a logger utility class, and do you know what it prints when you call logger.debug("some message")? It prints DEBUG.

This code handles an HTTP request, and stuffs the body of the request into the database; here's hoping that it's a well formed request. Somebody's got a lot of faith in their front end. WHat's interesting about this one is they've tried two different ways of copying the request object into the database, the first one focusing on making sure they only copied non-inherited properties, and the second just YOLOing the data into the database.

Now, this particular segment goes through their ORM to write data into the database. But not all the code does that. Many places write data through direct SQL, and guess what happens there: SQL injection vulnerabilities.

You may also notice that this function doesn't do any authorization checks, which is fine, that should be configured in the middleware. Should be- but isn't. Most endpoints have no authorization checks at all. Even the endpoints that do, like their admin API, have copies of the same endpoint with no authentication configured.

[Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.
Remy Porter