sec. vuln.: brackets {{ and }} in URL can trigger template engine
capc0 opened this issue ยท 10 comments
If any request constains {{
or }}
in the URL, e.g.
http://url.tld?param={{dummy}}
the following log always errors.
app.use(expressWinstonLogger({
winstonInstance: logger,
msg: (req: APIRequest, res) => {
return `HTTP ${req.url}`;
},
colorize: true,
}));
with
ReferenceError: dummy is not defined
at eval (lodash.templateSources[4]:9:10)
at C:\Users\Simon\Documents\Development\api\node_modules\express-winston\index.js:160:46
at ServerResponse.res.end (C:\Users\Simon\Documents\Development\api\node_modules\express-winston\index.js:419:23)
at ServerResponse.send (C:\Users\Simon\Documents\Development\api\node_modules\express\lib\response.js:221:10)
this might also cause some security issues.
http://url.tld?param={{console.log(1)}}
actually prints 1
in the console...
How can I disable the template engine, since I provide my own msg
function?
Is this lib still maintained? This is a not-so-small security issue...
requests like url?{{process.exit(1)}}
will kill APIs
Sorry I missed this @capc0, that looks pretty bad indeed. We have a plan to move our lodash dependency to a separate library in the 5.x series, is currently the source of most of the security issues in the library, what do you think we can do for now?
I suggest removing the usage of the template engine when a custom message function is declared.
So alwas return m
in https://github.com/bithavoc/express-winston/blob/main/index.js#L154 and so avoid running https://github.com/bithavoc/express-winston/blob/main/index.js#L160
Maybe create a property within loggerOptions
to disable the feature.
Hey @bithavoc is this package still maintained?
short answer: yes
long answer: we had some appointed maintainers but seems most have faded away, I can take the helm back.
I can't fix this right now and the typescript + lodash split seems like a long effort.
I can accept a PR to fix this specific issue though.
Hey @bithavoc is this package still maintained?
short answer: yes long answer: we had some appointed maintainers but seems most have faded away, I can take the helm back.
I can't fix this right now and the typescript + lodash split seems like a long effort.
I can accept a PR to fix this specific issue though.
Okay, thanks for your reply @bithavoc!
I have a quick fix in mind, that would basically follow the suggestion @capc0 made.
Can you give me write permissions to this repo so that I can open a PR? Thanks!
Hey @bithavoc is this package still maintained?
short answer: yes long answer: we had some appointed maintainers but seems most have faded away, I can take the helm back.
I can't fix this right now and the typescript + lodash split seems like a long effort.
I can accept a PR to fix this specific issue though.Okay, thanks for your reply @bithavoc!
I have a quick fix in mind, that would basically follow the suggestion @capc0 made.
Can you give me write permissions to this repo so that I can open a PR? Thanks!
you don't need write permission to the repo, you're welcome to fork and send a Pull Request
Hey @bithavoc is this package still maintained?
short answer: yes long answer: we had some appointed maintainers but seems most have faded away, I can take the helm back.
I can't fix this right now and the typescript + lodash split seems like a long effort.
I can accept a PR to fix this specific issue though.Okay, thanks for your reply @bithavoc!
I have a quick fix in mind, that would basically follow the suggestion @capc0 made.
Can you give me write permissions to this repo so that I can open a PR? Thanks!you don't need write permission to the repo, you're welcome to fork and send a Pull Request
Sure, here's the PR:#284
This is a critical security vulnerability. Like CVE-level critical. Arbitrary remote code execution means an attacker could do anything on your server, potentially without you knowing. I won't share code snippets for security reasons, but I have confirmed that a vulnerable server could be exploited to read/write arbitrary files, read/write to any databases it has access too, and exfiltrate the full environment variables of the host. I'm sure there's more a sophisticated hacker could do as well. This needs to be fixed and a security advisory published ASAP.
@bithavoc The comment from @thislooksfun is on point. Please see if you can accept that pull request. It can hardly make the situation worse unless it's actively malicious.