Auditing Code (security wise)

I don't know exactly if this is required or not, but would it be an idea if we could get a full audit of the core?

by this I mean find a professional or someone with enough knowledge to go through the core and audit the code for security issues etc.

would it be worthwhile?

it could be expensive i know, but the cost could be justified providing it isn't going to break the bank as to speak.

just an idle thought on my part, but thought it worthy of discussion.

Citaat:

ABSOLUTELY!!!

just done a quick audit myself.

well i say quick, but it actually took me well over 2 hrs to complete, and that was only a very basic audit looking for 1 particular issue.

issue i have dealt with today is to make sure that header redirects 'header() & redirect_header' are all exited properly with exit();

not an issue for browsers etc, but if the pages were to be viewed via say telnet then it could become an issue as telnet does not understand header functions, so essentially the header redirect is ignored and the rest of the page will be continued on. exiting the script with exit(); after each redirect will prevent that from happening. it protects from those systems like telnet that don't understand the header redirect function.

nothing tedious, just a simple check.

i'll continue with this as i go along, obviously the more complex coding and vulnerabilities will be beyond my knowledge, but for those that i know about, i'll fix as i go.

Yes, yes, yes!

There are many tools that may assist us in this effort
* http://phpsec.org/projects/phpsecinfo/index.html
* http://www.nessus.org/nessus/
* http://www.security-database.com/toolswatch/PHP-Security-Scanner-1-2-added-to.html

I am not skilled enough in PHP or JS to spot vulnerabilities, so I can only start with tools like these.

Very nice initiative Vaughan !

Keep up the good work !

And thanks for the links steve.

Citaat:

Do you mean executing the scripts from the command line, as opposed to via HTTP?

I attended a one-day seminar on web applications security last year. I'll dig out my notes and post them.

yes dave :) but i used telnet as just 1 example.

thanks for the offer of your notes, any information that can help improve security is a bonus.

As promised. here are my notes:

Notes from SPI Dynamics workshop, Richmond, VA, 2007-04-17

This free workshop was obviously intended to encourage people to buy SPI Dynamics' web security products. But the presenter, Brett Sagenich (brett.sagenich AT spidynamics DOT com), was a security engineer, not a salesman, and he provided much information of general use. His point was that web applications present numerous potential security vulnerabilities. He demonstrated actual techniques used by attackers. The reason that he discussed these details is that while they can all be addressed by proper software design, this is very labor-intensive, while SPI Dynamics' tools perform automated detection for these vulnerabilities.

Specific vulnerabilities discussed:

1. Extraneous files such as readme's, documentation, old files
- Provides info to attackers
- Old versions of scripts may have unpatched security issues.

2. Unvalidated user input

3. Visible error messages
- May reveal information useful to attackers
- Software in use
- File system paths
- Variation in error messages in response to an attacker's input can guide him.

4. SQL injection
- iterative (trial and error)
- with error messages
- blind (based on displayed output or presence/absence of output)

5. Session hijacking
- Exploit spoofable session ID, customer ID, etc.

6. XSS (or CSS, cross-site scripting)

---

I can elaborate on some of these items, if there are questions. I know that "Unvalidated user input" is a problem often encountered with XOOPS.

---

By the way, here's a good reference I found on this topic: Open Web Application Security Project (OWASP)

Few utilities I've found:

http://developer.spikesource.com/projects/phpsecaudit/

http://www.dragoslungu.com/2007/10/30/pixy-is-a-free-php-code-audit-tool/

http://www.php-mag.net/magphpde/magphpde_news/psecom,id,27497,nodeid,5.html

http://www.darknet.org.uk/2007/11/skavenger-source-code-auditing-tool/

nice reading, very exhausting tho..

thanks for those dave & dave.

with regards to the conversation I had with Dave last night, it is apparent that the exit(); after redirect_header() is not required as redirect_header function is already terminated with exit(); in the function itself. I'll remove the extra exit(); in a bit for consistency.

my next task that I will look for is in the mysql query statements. In particular making sure that all values in the sql query itself are quoted '' regardless of datatype, this includes alphanumeric values aswell as integer values. this will help decrease SQLi attacks.

for example >



is very vulnerable (i am not ignoring sanitation here, but just the above is for example of why quotes are necessary)

in the above, if a user provides a value of "1 OR 1=2" the above query then becomes >



which means that an attacker could then retrieve an arbitrary row from the db bypassing the password itself.



I think maybe a good idea for me to keep using this thread to write down my intentions and explain the reasoning, + if anyone else would like to also offer suggestions, it's a good place to keep this discussion in 1 place.

Should we also review where queryf is used in key modules, as this is often misused by developers who want a quick way to access core tables.

Herko

Citaat:

yes, i think that should also be done aswell being as queryF bypasses the text sanitizer etc. good point :)

I don't think queryf bypasses the text sanitizer; it just allows non-SELECT queries, such as UPDATE and INSERT, to be done when processing a GET request. But I agree that queryf should only be used in special situations when it's really needed.

ahh yes you are right Dave :) i knew it was to do with the GET requests but wasn't 100% sure if it bypassed the sanitizer or not.. thinking about it now tho, i can't understand why i thought it would do as that would be nightmare for abuse.

Citaat:
Correct. XOOPS database factory automatically prevent UPDATE and DELETE query to be used in a GET request. So if you absolutely need to use on of these queries in a GET request, then you would need to use queryF().

For example, updating the counter of an article when a user gets to the page would need a queryF as the user is not accessing the article via a POST request...

But indeed, queryF needs to be used with extra care. The concept behind is that all queries that changes the database need to be within a POST request.

Another practice to watch for - use of unsanitized $_GET variables.

Citaat:

That applies to all user input: GET, POST, cookies, files, and probably session variables too.

Also let's put some attention to the potentially dangerous:


ICMS checks the superglobals for being present in requests but still it is better to prevent vulnerability by not using commonly known risky programming methods.

Absolutely agree.

Can someone proceed to a search on all the ICMS base code to look for these :

Citaat:
or
Citaat:
and submit a bug request about this ?

ANything else we should be lookig for ?