Beefy Boxes and Bandwidth Generously Provided by pair Networks
go ahead... be a heretic
 
PerlMonks  

Blackbox implementation, but....

by IndyZ (Friar)
on Apr 29, 2002 at 03:08 UTC ( [id://162754]=perlmeditation: print w/replies, xml ) Need Help??

Earlier today a friend asked me to help him install a fairly simple CGI that is similar to the infamous formmail.pl from Matt's Script Archive. He was having trouble with the server throwing "Internal Server Error" messages and since the permissions looked right and we did not have access to the error logs, I dove into the source to find the problem.

Well, the code was a mess. No strict, no warnings, no CGI module. They had hand rolled their own parser for postdata parameters. I modified it to the point where it uses the CGI module, warnings, taint mode, and strict. I also hardened a few sections of the code that might have been exploitable.

I then went and made quite a few other improvements because, well, I was bored. The header (which comprises all of the documentation of the script) says, "there are no restrictions on this script," and "this script may not be redistributed without this header." I sent an email to the company that distributes the original script and asked for permission to distribute my changes even though I felt that the documentation had given me the right to do so as long as I included the header. The company does not even claim a copyright on the script.

You can probably guess where this is going. They sent me a polite email telling me in no uncertain terms that I could not make my changes publicly available. I have reduced a 250 line script to less than 200 lines. In the course of this I have significantly modified or written from scratch more than half of the current source code. I would consider rewriting the script from scratch in the spirit of Not Matt's Scripts, but I'm afraid that I am already too familiar with the code to create a legitimate independant implementation. Any advice?

--
IndyZ

Replies are listed 'Best First'.
Re: Blackbox implementation, but....
by belg4mit (Prior) on Apr 29, 2002 at 03:34 UTC
    Well had you not asked, and gone with what they said in the file, it sounds like you could have distributed it. Now, well it's their word vs. their word. And I'm afraid their more recent word probably wins out.

    NMS has a formmail wouldn't it have been easier to adapt to that in the first place?

    --
    perl -pew "s/\b;([mnst])/'$1/g"

      NMS has a formmail wouldn't it have been easier to adapt to that in the first place?

      It probably would have been but my friend had already done a lot of setup regarding that script and my first round of changes took less than a half hour. Beyond that it was just me spending an idle Sunday trying things and playing around. It's my favorite way to learn and, for me at least, is more effective than reading other people's code.

      --
      IndyZ

Re: Blackbox implementation, but....
by cjf (Parson) on Apr 29, 2002 at 08:10 UTC
    a fairly simple CGI that is similar to the infamous formmail.pl from Matt's Script Archive

    How is the script you modified different from the NMS Formmail script? If the company who created the script is refusing to let you make your changes public, just use the NMS script, and modify it slightly if necessary. In my experience it's better just to stay out of such messes with incompetent companies.

    Also keep in mind that you may have fixed most of the problems, but adding strict, warnings, and taint checking doesn't make a piece of code secure and reliable. There have been more people working on the NMS scripts for longer, and although Re-inventing wheels is not always bad, doing it does have its downsides.

      The biggest difference is that the script is seperated from the html and uses redirects to push the user around to success/fail pages.

      As I said in my last post, after looking at the source I wouldn't have even considered using it, but this friend had already built his HTML around it and would have figured out how to make it work without me (the shebang line was wrong), so I figured I might as well do what I could. I am fully aware that using strict, the CGI module, warnings, and taint do not make a script secure. However, it probably won't hurt. In addition, I made changes to the script's broken input validation and a few regexes that weren't as solid as the original coder probably thought they were. In the end I know for a fact that I closed one security hole and I am reasonably sure that I didn't introduce any new problems.

      --
      IndyZ

        In the end I know for a fact that I closed one security hole and I am reasonably sure that I didn't introduce any new problems.

        There are a lot more people who are reasonably sure that the NMS scripts don't introduce security problems :).

        Considering the problems with the creators of the original faulty script your friend was using, I'd recommend you either rewrite the script from scratch or adapt the NMS ones (separation of code and HTML is usually a good thing). If you choose to rewrite the script, it will probably be more work and will result in a less solid script than if you adapted one of the NMS ones.

        Your call though, check Re-inventing the wheel is a 'Good Thing' for more opinions on the issue.

        In the end I know for a fact that I closed one security hole and I am reasonably sure that I didn't introduce any new problems.

        i get the impression that this is a script that the company makes available to the public. if you know for a fact that you closed a security hole, it would logically imply that the version the company is distributing contains a security hole. i think the responsible thing to do would be to publish the vulnerability on bugtraq or some other appropriate forum.

        if other people are using it, publishing the vulnerability may encourage them to either remove it from their servers or fix it. it would also perhaps encourage the company to integrate some of your changes and maybe consider being a little nicer to people who send them patches in the future.

        anders pearson

Re: Blackbox implementation, but....
by Juerd (Abbot) on Apr 29, 2002 at 13:58 UTC

    we did not have access to the error logs, I dove into the source to find the problem.

    Next time, try some CGI::Carp solution, or try the script on another machine.

    Well, the code was a mess. No strict, no warnings, no CGI module. They had hand rolled their own parser for postdata parameters. I modified it to the point where it uses the CGI module, warnings, taint mode, and strict.

    Unfortunately, this is an unwritten CGI script coding standard. However, it is not always a problem. If it works and is maintainable and secure, it does not really matter much.
    Messy code can be maintainable if it's documented well and set-up logically. Tools like perltidy can fix awkward indentation, so if that is the only problem, give it a try.
    Code that doesn't use strict is not wrong per definition. Without strict, you're not forced to code without lexicals and without strict, no-one forces you to use symbolic references all over the place. Given that all code that does run with use strict also runs without, you should understand that code without it isn't always bad.
    Warnings aren't errors because they are not fatal. They warn the programmer, but have nearly no meaning for the user. Of course, like strict, it's better to ensure that the code doesn't emit warnings and keep -w or use warnings in there, but it's quite common to remove it once code goes into production. Warnings are not important when the development cycle is over, and one can do without. (He who does will have more trouble finding bugs, but warnings are not necessary to create a good application.)
    The CGI module is not a good module in my opinion. You complain about the code not using strict, but I think you missed that CGI.pm doesn't use strict either. Furthermore, CGI.pm is bloated, slow and parses in a way that does not please me. It is, however, very compatible with a lot of things.
    Having your own parser can be a good thing, for example if you need to know the difference between query string and POST data. CGI.pm does not handle this the way it should: posted forms can have a query string too, and in many cases it's bad to mix the two.

    I would consider rewriting the script from scratch in the spirit of Not Matt's Scripts, but I'm afraid that I am already too familiar with the code to create a legitimate independant implementation. Any advice?

    Just Do It, but do so blindly, without looking at the original code. There is probably nothing specific to the algorithm.

    Please note that I am not against strict or warnings, or think code that uses CGI.pm is bad. I'm just noting that bad coding styles don't always lead to bad code.

    - Yes, I reinvent wheels.
    - Spam: Visit eurotraQ.
    

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlmeditation [id://162754]
Approved by rob_au
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others perusing the Monastery: (8)
As of 2024-03-29 08:19 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found