vlachoudis / bCNC

GRBL CNC command sender, autoleveler and g-code editor
GNU General Public License v2.0
1.54k stars 528 forks source link

Correct ReDoS Issue #1721

Closed tatarize closed 1 year ago

tatarize commented 1 year ago

The parsing for svg_elements as presented in the original suffers a potential reDoS in the parsing of the font-shorthand attribute. The more copies of the word "normal" are added the more backtracking needs to be performed requiring days of processing for a few dozen copies. Regular Expression Denial of Service Attack.

--

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg>
    <text
       font="normal normal normal normal normal normal normal normal normal normal normal normal normal normal"
       id="textobject">reDoS</text>
</svg>

Contains no relevant objects for bCNC but would take like 5 minutes to open.

Harvie commented 1 year ago

Was this fixed in upstream library? i don't really want to introduce differences from upstream, so we can upgrade easily when upstream gets new release...

tatarize commented 1 year ago

Yes.

https://github.com/meerk40t/svgelements/commit/fad014e37c32481ab29d760780816f2c0b7f004e

Is basically the same commit and was included in the version update to 1.7.0 of svgelements.

But, also, a lot of things are fixed in the upstream library. The upstream library is 1.9.0 version now. This was 1.4.2. It's mostly maintenance (I did add marginal saving ability) changes though. A few of them would matter since they could crash here during a load (fragment files like <g><path></g> without svg header, css style block with comments, segmenting with numpy (as is done here) of path like M0,0M1,1z), though a lot of these are things like proper internal CSS component ordering, numerical stability of arc bulge. Yet others include a couple things like corrections for Use to work and load correctly (it wouldn't have crashed, it could just give you a wrong answer).

The upstream library gets a lot of upgrades but is version python 3 compat only (since py2 eol), with a couple version that were brought into python 2/3 mostly to update here, see https://github.com/meerk40t/svgelements/releases/tag/1.4.3

In this case, the ReDos issue might get flagged by some security checkers since you can technically send a SVG with the name of a font that will cause catastrophic backtracking and lock up for near infinite time (depending on how many normals are sent).

tatarize commented 1 year ago

The disagreement between this an main is that clean_code_dev replaced some of the % with fstrings.

flag_re = re.compile("|".join(f"(?P<{p[0]}>{p[1]})" for p in flag_parse))

This code is only compatible with Python 3.6+

tatarize commented 1 year ago

In fact after commit https://github.com/vlachoudis/bCNC/commit/6905d8ea64c5f9e6fe78a587e18558db98506bcf

The base code looks to be Python 3.6+ only. If this is the case I can happily just import svgelements 1.9.0 in there, rather than this little piecemeal change. A direct include of the library on pypi locked to a version would be markedly simpler than what exists now. It was largely a consequence of the 2/3 compatibility lag.

tatarize commented 1 year ago

This PR is unneeded and subsumed.