I'm playing with uploads, so I'm wondering if this route is safe to use, because if you somehow break off from the md5
function path, then you can control the path (if in TypeScript null byte is a thing).
Note: Snyk
denotes this as an LFI
issue!
Finally, my question is this vulnerability or the Snyk
just evaluates this way (because of variable names)? And also, is there any other way except the infamous null byte to bypass extensions?
The pseudocode is written in TS.
// ? This is pseudocode and it is not 100% valid.
router.post("/profile", async (req: Request, res: Response) => {
if (img.mimetype === "image/png" || img.mimetype === "image/jpeg") {
if (img.size < 1048576) {
var user = { userName: "UserControlledInput", password: "password", email: "email" }
if (user) {
let imgName = md5('prefix' + user.userName);
let imgPath = "assets/avatars/" + imgName + "." + img.mimetype.split("/")[1]
fs.writeFile(path.join(__dirname, "../jekyll/_site/" + imgPath), img.buffer, async (err) => {
if (err) {
console.warn("Error:", err);
} else {
user.save_to_database()
return res.send({ message: "Success!" });
};
})
} else {
res.send({ message: "First you must login, Not allowed!"})
};
} else {
return res.send({ message: "Not allowed, files larger then 1MB." });
};
};
});
md5
function - BTW, don't use MD5, just don't, there's no reason to do so, use something modern - encoded? If it's treated directly as UTF-8 or UCS-2/UTF-16 or some such, there is a risk here. Not a huge one - MD5 is broken but AFAIK nobody has a feasible attack that allows finding a partially-constrained preimage that hashes to an arbitrary digest - but it is in theory possible for the file name start with a bunch of "../" triplets. If it's encoded as hex or similar, though, that can't happen.md5
function is a package. You can try it yourself and it's a user-controlled field, so for the example it would be "e70503706bc80fbf2d346531baa4f3bc". --- Yeah, you are right. I should use something different, but I thought it was only a filename. What would you recommend to use? (just encoding or hashing) -- Anyway, appreciate your response.Synk
evaulates it that way because user input is somehow used to construct a local filepath. I don't see any way how a malicious user can manipulate that to include an actual local file path. Does not look vulnerable (to LFI) to me.