I’m working on a side project where I accept a post call into an end point and then push that data into a user record.
My starting point was based on a Udemy class where the instructor walked through a project that tied an Express app into Mongo DB. Here’s the user schema (../models/user.js):
const mongoose = require('mongoose');
const userSchema = new mongoose.Schema({
firstName: {
type: String,
required: true,
trim: true
},
lastName: {
type: String,
required: true,
trim: true
},
entitlements: {
type: String,
},
}, {
timestamps: true
});
So, a first name, a last name, and some kind of entitlements field that we don’t want to have filled out by the user–we want to apply some kind of business logic and fill that in ourselves.
Here is the user router (../routers/user.js):
const express = require('express');
const router = new express.Router();
// Import Model
const User = require('../models/user');
// Create a new user
router.post('/user', async (req, res) => {
try {
//Create the user
const user = new User(req.body);
await user.save();
res.status(201).send(user);
} catch (error) {
res.status(400).send({Error: error.message});
}
})
Here is my app.js (which is called by index.js):
const express = require('express');
require('./db/mongoose');
// Routers
const userRouter = require('./routers/user');
const app = express();
// Options
app.use(express.json());
app.use(userRouter);
module.exports = app;
Here is my ./db/mongoose.js file:
const mongoose = require('mongoose');
mongoose.connect(process.env.MONGODB_URL, {
useNewUrlParser: true,
useCreateIndex: true,
useFindAndModify: false,
useUnifiedTopology: true
});
Here is my index.js:
'use strict';
const app = require('./app');
const port = process.env.PORT;
app.get('', (req, res) => {
res.send("Nothing here, but it's up and working...");
});
app.listen(port, () => {
logger.logInfo("Server is up on port " + port);
})
As you’ve no-doubt guessed, this is a very trimmed down version of my actual app. I believe that I’ve got all of the relevant pieces such that this would run, but if I’ve missed something I apologize–the point of this post should still come through.
The issue I noticed is that with what I’ve got above, it’s actually possible for the user to pass an ‘entitlements’ property on the request and it will push that through to the database. Obviously, that requires that the person trying to exploit the loophole figures out that you’ve got an entitlements field on your user record. Then, they have to figure out how you’re representing elevated access in that string field (or however you’re storing the thing that you don’t want users setting themselves).
That’s all very unlikely, but it would be foolish to depend on it not happening.
As far as addressing the gap, my first instinct was to go through and delete off the attributes on the request that I don’t want the user to be able to set. Something like this:
const express = require('express');
const router = new express.Router();
// Import Model
const User = require('../models/user');
// Create a new user
router.post('/user', async (req, res) => {
try {
//Remove any protected fields
delete req.body.entitlements;
//Create the user
const user = new User(req.body);
await user.save();
res.status(201).send(user);
} catch (error) {
res.status(400).send({Error: error.message});
}
})
That does the trick, but is asking for problems down the road. Each time I add a new ‘protected’ field that I don’t want users to be able to set, I’ve got to remember to come back here and remember to delete it off of the request body. I can virtually guarantee that I’ll forget to do that at some point.
The better option is to make sure that I only submit ‘non-protected’ fields to the database. Something like this:
const express = require('express');
const router = new express.Router();
// Import Model
const User = require('../models/user');
// Create a new user
router.post('/user', async (req, res) => {
try {
//Copy over acceptable attributes so that call can't populate protected fields
const userObject = {
firstName: req.body.firstName,
lastName: req.body.lastName
}
//Create the user
const user = new User(userObject);
await user.save();
res.status(201).send(user);
} catch (error) {
// console.log(error.message);
res.status(400).send({Error: error.message});
}
})
Obviously, that’s not perfect. If I add a new ‘non-protected’ field, I’ve got to remember to come back here and add it to the list of the fields that are being copied over.
The plus side is that failing to remember to do that will cause things to break things right away, which will cause me to come back and fix the bug. Failing to remember won’t result in a vulnerability.
