formaction attr allowing javascript in Cleaner() if safe_attrs_only is set
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
lxml |
Fix Released
|
Medium
|
Unassigned |
Bug Description
Python : sys.version_
lxml.etree : (4, 5, 2, 0)
libxml used : (2, 9, 10)
libxml compiled : (2, 9, 10)
libxslt used : (1, 1, 34)
libxslt compiled : (1, 1, 34)
The following script creates a form with a button with formaction which still allows XSS through when clicking the button.
```
from lxml.html.clean import Cleaner
cleaner = Cleaner(
forms=False,
safe_
)
cleaner.
```
However, this same kind of idea doesn't apply to action on the form which is somewhat equivalent:
```
In [1]: cleaner.
Out[1]: '<div><form id="test"
In [2]: cleaner.
Out[2]: '<div><form id="test" action=
```
safe_attrs_only is an unsafe setting to disable but it seems to respect the javascript setting so I would argue that formaction should be added to the list of attributes that are removed by the javascript setting.
CVE References
Changed in lxml: | |
milestone: | none → 4.6.3 |
status: | Confirmed → Fix Committed |
information type: | Private Security → Public Security |
Changed in lxml: | |
status: | Fix Committed → Fix Released |
I believe the issue is that lxml isn't treating a button as a "link" and not running formaction through `doc.rewrite_ links() `
https:/ /github. com/lxml/ lxml/blob/ 34aa8896f99f93a 43f3c61fc66beb4 59ce163acd/ src/lxml/ html/clean. py#L300
From there iterlinks() is called which doesn't see a button as a link.
I'm not sure if you would want to modify iterlinks() or add a special case for button in the clean function.