1

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." });
    };
  };
});
4
  • How is the output of your 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.
    – CBHacking
    Nov 22, 2022 at 8:53
  • @CBHacking - I forgot to mention that the 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.
    – Kiwi501987
    Nov 22, 2022 at 17:46
  • OK, hex-encoded, so that's not a problem. Still recommend something from the SHA2 or SHA3 family. Since you don't really need it for security per se you could go with the shortest 224-bit (28-byte) digests, though usually people opt for 256. It's not even that I expect MD5's cryptographic weaknesses to be relevant here (even without the hex encoding); it's just an obsolete algorithm. Also, any security scanner you run on your code will yell at you about it.
    – CBHacking
    Nov 24, 2022 at 6:29
  • 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.
    – zyked
    Dec 21, 2022 at 18:47

0

You must log in to answer this question.

Browse other questions tagged .