In my opinion, all his comments show is his arrogance. This argument
again comes down to programming style. Look if he is really anal and
feels the need to define error/fail states for every system call, even
ones that call defined functions, stated variables, or static files,
then he can go for it! However it is perfectly acceptable not to in
most cases. Where it is agreed to be a best practice, is where the call
uses a variable or function that is dynamically generated from user
input, as you see in the snip below. Which by the way is from the ASSP
code line 5705.
open(F,"<$file") || mlog(0,"couldn't open '$file' for mail
report");
(Note - You will notice the < in front of the file name setting
it for explicit write which means it either opens the file for write
or fails in which case you get the trailing error message. This secures
this call against backwards file traversal attacks as there is only
one expected result. You find that this type of check is common
where needed in ASSP as it is a standard coding practice.)
However I don't see all system calls as a security risk in and of
themselves. It totally depends on the system call and how it is used.
The one the commenter makes reference to is a standard file write
function, not exactly a high risk call in most cases, but of course the
level of risk really comes back to how is the system configured? It
doesn't matter if you have a stateful error check or not if they are
calling the open function to a static file that is critical to the
operation of the application. Depending on the point in the process it
is writing it could still be used to inject problems even on a "known
fail state" if local user rights or ACL's are improperly set . Now if
you are using a set call of some kind and are running with elevated
privileges then yes it could be a problem. But that is why this is a
server app and should only be used by experienced users who know to run
it in a secure space, i.e. chroot, or with limited user rights if on
Win32.
Comments about code quality need to have context. His comments are like
saying that you need a warning light in your car to tell you the engine
is missing. No cars come with that light, why? Because they assume that
car owners will realize their car won't run without an engine.
Generally system calls are not considered to be a big risk unless passed
input from a user or external process, in which case the guidelines for
constraining user input and instituting system call "known good return
value" checks should be followed. For those unfamiliar with what they
should be, there are good resources at www.owasp.org and www.sans.org
that not only states agreed upon best secure coding practices but also
gives guidelines for running applications securely.
Of course application security is an ongoing task, but I think if you
were to spend time checking (which we all should do if we get a chance)
you will find that as a general rule, where necessary, ASSP code has the
appropriate measures in place and conforms to the industries stated best
practices for programming as put forth by OWASP and SANS.
The comments by this person show only one thing really, and that is that
he obviously has a chip on his shoulder concerning ASSP. Could it be
that he doesn't like how popular it is becoming? Maybe he is biased
toward another piece of software?
I don't know what his problem is, but I do know that he is wrong about
the insecurity of ASSP.
Ged
-----Original Message-----
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Charles
Marcus
Sent: Saturday, January 27, 2007 2:42 PM
To: Questions and Answers for users of ASSP Anti-Spam SMTP Proxy
Subject: Re: [Assp-user] Questions regarding code-quality and
(in)securityof ASSP...
> No, but they can be a highly opinionated bunch of people who are
> highly dissatisfied with just about everything they haven't thought of
> or implemented themselves.
I am beginning to agree, but I was able to get the guy who claimed to
have looked at the code to be precise in his criticism. Now, below are
two examples that jumped out at him that he is using to base his
criticism of ASSP in general on. What he says SOUNDS reasonable, but
since I am not a programmer, I don't know if it has technical merit or
not.
I'd like to get comments on it from anyone who understands proper
programming technique, and then I'll let this thread die.
> I simply refuse to engage in pointless arguments with them. I use ASSP
> in a 3 server production environment and am very happy with the
> solution.
Personally, I don't consider this argument/discussion pointless. I am
very interested in learning more about the s/w that I use on a daily
basis, and when someone has what *appears* to be genuine criticism, I
like to find out more.
Again, Fritz, I'm not complaining, I'm not criticising - ASSP works
great for me. I'm just curious to hear what those more knowledgable -
both with programming in general, and programming specifically in perl -
have to say about the concerns this guy took the time to be specific
about.
So that said, here is what he had to say:
************************************************************
Let's go back to this fragment, found relatively early on in the code:
open(F,"<$base/assp.cfg"); local $/;
(%Config)=split(/:=|\n/,<F>); close F;
This is the sort of thing that earns someone an instant "F" on a college
programming assignment. It's completely unforgivable in a piece of
software used in a security role.
Why? Because it blithely presumes that the open() system call will
succeed. It doesn't make the slightest attempt to recognize that it
might fail, let alone try to do something sensible if it does. (And
yes, it does the same thing with the close() call, and yes, sometimes
close() calls fail.)
Any time that any program makes any system call (or subroutine call) it
should check what's called "the return value". The possible return
values are documented, and usually at least one is reserved to indicate
failure -- i.e. the call didn't work. Sometimes more than one is
reserved, and which one is returned indicates not just failure, but what
kind of failure.
So part of sound programming practice is to check the return value from
a system or subroutine call and make sure it's something indicating
success BEFORE trying to do something with it. (And if it's something
that indicates failure, then print out or log an error, try something
else, give up and exit, do something but just don't keep going on as if
nothing happened.) In this particular case, the file descriptor F is
blindly used EVEN IF IT'S GARBAGE.
Something along the lines of:
open(F,"<$base/assp.cfg") || die "cannot open ASSP configuration
file $base/assp.cfg, giving up\n";
would be at least an incremental improvement.
Now this isn't a subtle, hard-to-find, hard-to-exploit complex problem
that would take weeks to figure out. It's something I spotted on the
first read-through. Which tells me -- because this is not the first
piece of code I've ever read -- that because errors this obvious, this
egregious, this basic, have managed to survive through all the work
that's been done on the code, that it's very likely that other similar
errors, and probably worse ones as well, have also survived.
Oh look, here's another one just like it at line 1449:
if($pidfile) {open(F,">$base/$pidfile"); print F $$; close F;}
There are probably more of these -- I didn't bother looking. And that's
just open(). What about all the other system calls? All the other
subroutine calls (both to internal and external routines)?
The fact that simple things like this are horribly broken doesn't bode
well.
*************************************************************
--
Best regards,
Charles
------------------------------------------------------------------------
-
Take Surveys. Earn Cash. Influence the Future of IT Join
SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDE
V
_______________________________________________
Assp-user mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/assp-user
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Assp-user mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/assp-user
