ChangdongLi commented on pull request #68: URL: https://github.com/apache/freemarker/pull/68#issuecomment-647003296
BTW you can view the hardcoded version here https://github.com/ChangdongLi/freemarker/commit/99d7fa016d0d5677620b5d189727519175aaf153 Best Regards, Danny On Sun, 21 Jun 2020 at 00:27, Danny Li <[email protected]> wrote: > you are right. > The solution you mentioned is better than that pull request. > The data model content in our application is a hash map. Its keys are > fixed strings. The template doesn't allow to refer to other keys. Will this > data model content be safe? So far combined with the features disabled > via hard coding, our penetration tester said okay. > If we will be able to disable some features in the future official > FreeMarker version via the solution you mentioned, > Will this be enough for security? > Thanks > On 20 Jun. 2020, at 23:13, ddekany <[email protected]> wrote: >> >> Just to restate the problem. Official FreeMarker does have features to >> disable these on the level of the Configuration object. Your problem is >> that sometimes you don't have control over the Configuration instance. >> So, you started rolling your own in-house FreeMarker fork, that just >> doesn't have some dangerous features, and what you need is a way to achieve >> the same without actual forking. >> >> If we just have some static method, where you can fiddle with the >> Configuration instance, that perhaps can be too easily used for attacks. >> If others manage to call that static method (yes, then you already have >> some big problem, but still), they can make a previously secure system >> insecure. Also, it's not always easy to guarantee that your static method >> call happens before the relevant Configuration instance was created. >> Such uncertainty is especially undesirable when security depends on it. So >> maybe, we rather should have class with a fixed, predefined name, let's say >> org.apache.freemarker.monkeypatch.FeatureDisabler, and that's where you >> can do your thing. Because it's a documented, fixed place, it's also easier >> to figure out if such magic happens. The only problem is that as the >> presence of this calls is optional in FreeMarker, the system will still run >> if somehow it's not there anymore. For that though, you can refer to >> FeatureDisabler in your application own startup code, so your >> application will fail if that class is gone for some reason. >> >> Though I'm still not sure how do you intend to ensure that the data-model >> content itself is safe. Well, you might as well specify a whitelist in >> org.apache.freemarker.monkeypatch, though I'm not sure how realistic >> that is. >> >> — >> You are receiving this because you authored the thread. >> Reply to this email directly, view it on GitHub >> <https://github.com/apache/freemarker/pull/68#issuecomment-646993026>, >> or unsubscribe >> <https://github.com/notifications/unsubscribe-auth/AANACRXVK4NDAKSPB2QWXYTRXSYXJANCNFSM4OCN6PFA> >> . >> > ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
