Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When I use **while** not stop in the views #1155

Closed
parg-programador opened this issue Aug 24, 2019 · 16 comments
Closed

When I use **while** not stop in the views #1155

parg-programador opened this issue Aug 24, 2019 · 16 comments
Assignees
Labels
Bug This issue is an actual confirmed bug that needs fixing
Milestone

Comments

@parg-programador
Copy link

Hello guys, I'm trying to use the while statement in views, but I don't stop, even if the condition is met, look this:

            <div class="flexslider flexslider-thumb">
              <ul class="previews-list slides">
                <% qtd_img = 0 %>
                % for i in range(1, 11):
                    % if getattr(produto, 'imagem' + str(i)) is not None:
                <li>
                    <a id="imgthb_{{i}}" href="{{usar_webp(get_imagem('produtos', getattr(produto, 'imagem' + str(i))))}}" class="cloud-zoom-gallery" rel="useZoom: 'zoom1', smallImage: '{{usar_webp(get_imagem('produtos', getattr(produto, 'imagem' + str(i))))}}'">
                        <img src="{{usar_webp(get_imagem('produtos', getattr(produto, 'imagem' + str(i))))}}" alt="Imagem {{i}}"/>
                    </a>
                </li>
                    <% qtd_img += 1 %>
                    % end
                % end
                {{qtd_img}}
                % while qtd_img < 3:
                <li>
                    <a href="{{usar_webp('/static/images/branco400.jpg')}}" class="cloud-zoom-gallery" rel="useZoom: 'zoom1', smallImage: '{{usar_webp('/static/images/branco400.jpg')}}'">
                        <img src="{{usar_webp(''/static/images/branco400.jpg')}}" alt="Sem imagem"/>
                    </a>
                </li>
                  <% qtd_img += 1 %>
                % end
              </ul>
            </div>

In this code I try use the while instruction to show in minimal three images for the slide layout. But never stops, anybody know solve this?

@defnull
Copy link
Member

defnull commented Aug 24, 2019

There is actually an endless-loop bug in template parsing, so the template never gets to the point the while loop is run. The parsing already hangs up. Interesting!

@defnull defnull added the Bug This issue is an actual confirmed bug that needs fixing label Aug 24, 2019
@defnull defnull self-assigned this Aug 24, 2019
@defnull defnull added this to the Release 0.13 milestone Aug 24, 2019
@defnull
Copy link
Member

defnull commented Aug 24, 2019

It's the img-tag in the while loop. The unbalanced single-quotes in usar_webp(''...') confuses the parser. Python would throw a syntax error, but the template parser hangs up in a loop for a reason I do not know yet. Just remove the wrong single-quote and you are fine, but keep the issue open for now so we can fix the parser bug.

@parg-programador
Copy link
Author

Thanks!! You saved my day.

@parg-programador
Copy link
Author

It would be nice if the bottle.py informs this SyntaxError.

@defnull
Copy link
Member

defnull commented Aug 24, 2019

looks like we have a runaway regular expressions in the template parsing code. It is not an endless loop or a bug in bottle itself, but a regular expression that is extremely slow for certain inputs (catastrophic backtracking problem). Hmm, that's not that easy to fix.

@parg-programador
Copy link
Author

We could make a method of throwing a timeout exception when regular expressions take a long time to respond, so it would be easier to identify the problem.

I must have spent about four hours rebuilding the loop code and trying to make it work, when in fact the problem was one more quote in that part of the code. I got stuck on this because the last code I had typed was while loop, after that it stopped working, but I didn't realize the real problem.

@defnull
Copy link
Member

defnull commented Aug 26, 2019

Unfortunately, python re.Pattern methods do not provide a timeout feature or any safeguards against catastrophic backtracking. The usual countermeasures (possessive quantifiers or atomic groups) are also not supported by python. The only way would be to rewrite the regular expression to not have conflicting nested repeated patterns (I tried and failed), or to rewrite the parser to not use regular expressions the way it currently does.

If Bottle or SimpleTemplate would be a normal library I'd immediately start fixing this issue, but Bottle is a micro-framework and the template parser is less than 200 lines of code. Rewriting it to catch this edge case would significantly increase its size and complexity. I'm not sure this can be done without a complete rewrite of most of its logic :/ Also, this is the first time this bug is reported in more than 10 years of project history. For this reason, I currently tend to mark this as "won't fix" until someone comes up with an acceptable solution.

That said, these bugs can be found quite easily with a debugger or Ctrl-C debugging (stopping the hanging python interpreter and looking at the stacktrace). The stacktrace would have shown you that the interpreter does not hang within your template code, but within StplParser or more precisely, during a re.Pattern.search() call. That would not have helped finding the actual typo, though. Really nasty bug :(

@mschwager
Copy link

Can you point out which regular expression in bottle.py is having the catastrophic behavior causing this issue?

@mschwager
Copy link

I think I found it. I had an outdated version of bottle.py, but with the latest version I believe Dlint found the issue:

$ python -m flake8 --select=DUO138 bottle.py 
bottle.py:349:19: DUO138 catastrophic "re" usage - denial-of-service possible
bottle.py:3020:11: DUO138 catastrophic "re" usage - denial-of-service possible

The issue on line 349 corresponds to #1194, and I believe the issue on line 3020 is this issue. The line itself is:

_hsplit = re.compile('(?:(?:"((?:[^"\\\\]+|\\\\.)*)")|([^;,=]+))([;,=]?)').findall

Again we have catastrophic backtracking in the (?:[^"\\\\]+|\\\\.)* portion because the branch has overlapping characters. E.g.

>>> _hsplit = re.compile('(?:(?:"((?:[^"\\\\]+|\\\\.)*)")|([^;,=]+))([;,=]?)').findall
>>> _hsplit('"' + '.' * 64 + ';')
... Spins ...

I'm not sure about the exact logic behind the alternation or branch (|), but to prevent catastrophic backtracking we have to prevent the two sides of the branch from having overlapping character sets.

@defnull
Copy link
Member

defnull commented Jan 3, 2020

The issue is very similar to #1194: The expression should match a quotes string up until the first non-escaped quote. Similar logic, similar pattern, same backtracking issue. Maybe the same fix also works here: Removing the inner repetition. I'll have to test that.

@defnull
Copy link
Member

defnull commented Jan 3, 2020

Oh, that's bad. The _hsplit issue is not related to this bug (which is triggered by the expression used to parse templates), but from the header parser and MIGHT be exploitable (DOS) by an attacker.

@defnull
Copy link
Member

defnull commented Jan 3, 2020

Phew, that was close. Fortunately, _hsplit and _parse_http_header() is not actually used by anything in bottle. It's dead code. Not a security bug. Still fixable.

@defnull
Copy link
Member

defnull commented Jan 3, 2020

Regarding this issue, I think it is the line

_re_inl = r'''%%(inline_start)s((?:%s|[^'"\n]+?)*?)%%(inline_end)s''' % _re_inl

Again, the inner +? may simply be removed to fix the backtracking issue. But I think there are more issues in this huge expression.

@defnull
Copy link
Member

defnull commented Jan 3, 2020

Yes, that's it. This change solves the issue. Hurray \o/

@defnull defnull closed this as completed in 332215b Jan 3, 2020
@mschwager
Copy link

Nice! For the record, Dlint doesn't know how to find the _re_inl issue yet. It only finds string literals that exhibit catastrophic cases, not variables that are eventually passed to the re functions. This is a good improvement to track, though!

rpuntaie added a commit to rpuntaie/stpl that referenced this issue Apr 1, 2020
kujenga added a commit to kujenga/bottle that referenced this issue Nov 1, 2022
Related to bottlepy#1194

This backports the patch from 332215b to the 0.12 release branch.
kujenga added a commit to kujenga/bottle that referenced this issue Nov 1, 2022
Related to bottlepy#1194

This backports the patch from 332215b to the 0.12 release branch.

This fix can be validated using the following repl commands:

    >>> import bottle
    >>> bottle.template("""<img src="{{usar_webp(''/static/images/branco400.jpg')}}" alt="Sem imagem"/>""")
kujenga added a commit to kujenga/bottle that referenced this issue Nov 1, 2022
Related to bottlepy#1194

This backports the patch from 332215b to the 0.12 release branch.

This fix can be validated using the following repl commands:

    >>> import bottle
    >>> bottle.template("""<img src="{{usar_webp(''/static/images/branco400.jpg')}}" alt="Sem imagem"/>""")
@kujenga
Copy link

kujenga commented Nov 1, 2022

Hi @defnull , I've submitted this PR to back-port the fix here to the 0.12 branch: #1402

defnull pushed a commit that referenced this issue Mar 4, 2023
Related to #1194

This backports the patch from 332215b to the 0.12 release branch.

This fix can be validated using the following repl commands:

    >>> import bottle
    >>> bottle.template("""<img src="{{usar_webp(''/static/images/branco400.jpg')}}" alt="Sem imagem"/>""")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This issue is an actual confirmed bug that needs fixing
Projects
None yet
Development

No branches or pull requests

4 participants