in reply to Re: CGI scripts and NMS
in thread CGI scripts and NMS

Sure, I will. Guestbook is my current favourite... Check out this "ultra-leet" substitution: 
  s[
    (?: <!--.*?-->                                   ) |
    (?: <[?!].*?>                                    ) |
    (?: <([a-z0-9]+)\b((?:[^>'"]|"[^"]*"|'[^']*')*)> ) |
    (?: </([a-z0-9]+)>                               ) |
    (?: (.[^<]*)                                     )
  ][
    defined $1 ? cleanup_tag(lc $1, $2)              :
    defined $3 ? cleanup_close(lc $3)                :
    defined $4 ? cleanup_cdata($4)                   :
    ''
  ]igesx;

[download]
No comments to explain it. I understand it. If you'd asked me a year ago, I'd have had no clue. If the code is vanity code to prove how clever the authors are, that's GREAT. But, I don't believe that was their intention. Or at least, it's claimed it wasn't.

The particular bug that springs to mind was that you could wipe the entire guestbook from view using comment tags, and possibly invoke SSIs if they were enabled. I submitted a patch, and it was patched. Matt's code wasn't vulnerable to this.

Replies are listed 'Best First'.
Re: Re: Re: CGI scripts and NMS
by davorg (Chancellor) on Jan 25, 2002 at 18:54 UTC

    Personally I don't think that code it too complex. And I think it's better to show beginners slightly more complex but correct code than to show them the code in most existing CGI repositories.

    And as for bugs. Well, no-one's perfect. We're not saying that we are. All suggestions will be very welcome on the nms developers mailing list.

    --
    <http://www.dave.org.uk>

    "The first rule of Perl club is you do not talk about Perl club."
    -- Chip Salzenberg

[reply]

      Hmm, I would say that the code is *complex* however I would not say it is overly *complicated* - bearing in mind what the code is doing, I am sure that everyone is in agreement that whitelist based HTML filtering is a good thing (unless one is a skript kiddie trying to damage the website of course :). I have had a couple of hacks at doing the same thing using HTML::Parser and I think that would be just as 'orrible looking :)

      The thing here is that there is always going to be a conflict between the didactic aims of NMS and the needs to provide secure and robust code - in this case the latter concern has become foremost, on the other hand we have rejected changes that have seemed overly obfuscated and hopefully implemented the same stuff in a more clear manner.

      For myself I am delighted that people are finding security holes in the NMS programs - this is an OPPORTUNITY for us to make the stuff better. For myself I would hate it if the programs were being used by people and the only people who knew there were vulnerabilities were the crackers and skript kiddies. I can't speak for anyone else on the project but I know that I am not omniscient :)

      /J\

[reply]
Re: Re: Re: CGI scripts and NMS
by nickjc (Initiate) on Jan 25, 2002 at 21:47 UTC

    Yes, the code is a little terse (to say the least) but please consider the fact that we're implementing a whitelist-based HTML filter without the aid of HTML::Parser. The HTML filter code already accounts for half the lines in guestbook.pl, so anything that makes it longer has to be considered with care.

    I think you're mistaken about the HTML comment vulnerability, it didn't work when I tried your example. I've put up a page where you can apply input to the filter, please test any exploits you can think of at this test page.

[reply]
    A reply falls below the community's threshold of quality. You may see it by logging in.
Re: Re: Re: CGI scripts and NMS
by sheriff (Sexton) on Jan 25, 2002 at 17:38 UTC
    Specifically, you could trick the comment stripper by saying something along the lines of: 
    <!<!----

    [download]
    Update:
    Wasn't clear here...
    Consider: <!-<!-- -->- instead. The substitution will remove the middle comment tag, and thus create a new one.
[reply]
[d/l]
[select]

In Section Meditations