We could simply add a warning for writes to any property named innerHTML that is suppress-able like any other warning. Then we can at least say you were warned.
I don't think we want to try to wrap innerHTML setters and guarantee that you can't possibly write a Royale app that can be exploited. HTH, -Alex On 3/16/20, 3:15 AM, "Harbs" <[email protected]> wrote: The danger is in runtime injection. > On Mar 16, 2020, at 11:49 AM, Carlos Rovira <[email protected]> wrote: > > Can we do special handling at compile time? React can't do this but we can. > If we detect <script> tags we can filter that or throw some warning or > error. > > just my 2.... > > El lun., 16 mar. 2020 a las 8:45, Alex Harui (<[email protected]>) > escribió: > >> >> >> On 3/16/20, 12:34 AM, "Harbs" <[email protected]> wrote: >> >> My thoughts: >> >> 1. We should avoid the use of innerHTML in framework code as much as >> possible. >> >> IMO, we only really need to avoid unfiltered writes to innerHTML. You >> should be able to read it. If a component writes it in a way that cannot >> be modified, it should be fine, but writing <img src=" + imageURL + "/> is >> not. >> >> 2. We should discourage the use of it in application code, although I >> think it’s not going to be very heavily used in general in Royale no-matter. >> >> OK, but how? Compiler warning? Remove innerHTML from the APIs? Use >> "dangerouslySetInnerHTML" like React? Always filter? >> >> 3. We are using innerHTML in parsing MXML and innerHTML is one of the >> default MXML properties. This seems to be safe as it's compile-time. >> >> I didn't understand #3. >> >> FWIW, I agree we should agree on how we treat innerHTML. I'm done for >> today, so will check tomorrow if there is more discussion. >> >> -Alex >> >>> On Mar 16, 2020, at 7:17 AM, Greg Dove <[email protected]> wrote: >>> >>> Before I forget, I believe we need to have a discussion about the >> use of >>> innerHTML inside the framework before we get to 1.0 release. A few >> of us >>> discussed this briefly in Slack a couple of months back, and we >> decided it >>> needed shared input/scrutiny on list. I'm only raising it here to >> achieve >>> that. >>> >>> Replying to this >>> I'm suggesting this needs attention, but I don't have any simple >> answer. >>> Please share your thoughts/ideas. >>> >>> Why does it need scrutiny? >>> While I can't see a direct security risk (it seems the main risk is >> in >>> combination with other things), it is considered at least a weak >> point, and >>> it sounds like it would probably cause a fail in a security audit of >> an >>> application. Aside from that, if there is real risk then it could >> affect >>> Royale's reputation, or that of Apache. >>> I think we are using innertHTML at various depths in a number of >> different >>> places from a quick 'search in files'. If values for setting these >> could >>> come from externally created strings that are not filtered for >> security >>> risks (e.g. <script> tags), then I think those would be the areas of >>> concern in a security audit. >>> >>> Others >>> I already know what React does for this, but I did not look >> elsewhere, so I >>> don't know what other frameworks do. With React they make it >> possible to >>> use innerHTML, but doing so in itself is a very clear reminder that >> it is >>> not a great thing to do. >>> See here : >>> >> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Freactjs.org%2Fdocs%2Fdom-elements.html%23dangerouslysetinnerhtml&data=02%7C01%7Caharui%40adobe.com%7C7ef4902d2bab4cf3b5cd08d7c992ec35%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637199505206353811&sdata=rLkJegbG9YyNMPktbBAEwqsgegRMwWK2gJH8tFJjRB8%3D&reserved=0 >> >> >> >> > > -- > Carlos Rovira > https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fabout.me%2Fcarlosrovira&data=02%7C01%7Caharui%40adobe.com%7C7ef4902d2bab4cf3b5cd08d7c992ec35%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637199505206353811&sdata=4t8ESBlqj6RmtKgjXr%2FHcJP%2FTAOtWYTd3Pp7snvtSpE%3D&reserved=0
