mashpie / i18n-node

Lightweight simple translation module for node.js / express.js with dynamic json storage. Uses common __('...') syntax in app and templates.
MIT License
3.08k stars 419 forks source link

[Discussion] Contrast reports path traversal vulnerability #486

Closed francislz closed 2 years ago

francislz commented 2 years ago

Contrast reports that the Accept-Language: en-us is vulnerable to Path Traversal since the value is used to access the en.json in the translations folder. According to contrast I might manipulate the this header in order to access the filesystem. I tested it out by manipulating the header but I seems that is a false positive. Just wanted to make sure by creating this discussion here.

mashpie commented 2 years ago

Thanks for pointing. If you have any further details to share or even a POC, please send them by email before disclosing. Meanwhile I'll test for traversal vulnerability by myself.

mashpie commented 2 years ago

So far I was not able to escape from the locales directory. Any provided locale (be it from accept-header, cookie, query parameter or code) has to match to one of configured locales, ie.:

const i18n = new I18n({
  locales: ['en', 'de'],
  directory: './translations'
})

The first locale will act as default for any "unknown" or manipulated locale. Thus, providing a locale like '../en' or '../de' or '../../' will result in using 'en' with reading/writing the file ./translations/en.json relative to process.env.PWD

Even without explicit config, i18n uses 'en' as default locale and 'locales' as default directory. Example:

const i18n = new I18n({})

can only use ./locales/en.json for any given locale.

I am about to add another test as prove for those cases. And I think there is still room for improving parameter sanitizing and filesystem operations in general.

francislz commented 2 years ago

@mashpie the tests on my side followed the same logic as yours and I was not able to escape from the locales directory as well. I think it is safe to assume its a false positive. Thanks for the fast response, btw :D

mashpie commented 2 years ago

@francislz Thanks for your effort, I am glad to hear that. Let's keep this issue open for me to further improve sanitizing.