Fork me on GitHub
Reply New Topic
2007/12/8 8:15:05
#1
Offline
Home away from home

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.


2007/12/8 8:17:49
#2
Offline
Home away from home

Re: Auditing Code (security wise)

Quote:

would it be worthwhile?



ABSOLUTELY!!!

_________________
JMorris (aka James Morris)
ImpressCMS Professional Services: INBOX International inc.
James Morris Online | Frolicking on the playground that is the Internet...

2007/12/8 12:19:52
#3
Offline
Home away from home

Re: Auditing Code (security wise)

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.

_________________
Live as if you were to die tomorrow, Learn as if you were to live forever

The beauty of a living thing is not the atoms that go into it, but the way those atoms are put together!

2007/12/8 12:40:37
#4
Offline
Home away from home

Re: Auditing Code (security wise)

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.

_________________

Steve Twitter: @skenow Facebook: Steve Kenow


2007/12/8 20:38:10
#5
Offline
Home away from home

Re: Auditing Code (security wise)

Very nice initiative Vaughan !

Keep up the good work !

And thanks for the links steve.

_________________
Marc-André Lanciault
Founder and CEO INBOX International inc.
Co-Founder ImpressCMS

2007/12/9 4:37:30
#6
Offline
Home away from home

Re: Auditing Code (security wise)

Quote:

the pages were to be viewed via say telnet



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.


2007/12/9 12:20:34
#7
Offline
Home away from home

Re: Auditing Code (security wise)

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.

_________________
Live as if you were to die tomorrow, Learn as if you were to live forever

The beauty of a living thing is not the atoms that go into it, but the way those atoms are put together!

2007/12/9 15:25:11
#8
Offline
Home away from home

Re: Auditing Code (security wise)

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) <http://owasp.org/>


2007/12/9 18:24:26
#9
Offline
Home away from home

Re: Auditing Code (security wise)



2007/12/10 4:04:17
#10
Offline
Home away from home

Re: Auditing Code (security wise)

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 >

DQokdWlkICA9ICRfR0VUWyd1aWQnXTsNCiRwYXNzID0gJF9HRVRbJ3Bhc3MnXTsNCm15c3FsX3F1ZXJ5KCJTRUxFQ1QgKiBGUk9NIHRhYmxlIFdIRVJFIHVpZD0kdWlkIEFORCBwYXNzPSckcGFzcyciKTs=

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 >

DQpTRUxFQ1QgKiBGUk9NIHRhYmxlIFdIRVJFIHVpZD0xIE9SIDE9MiBBTkQgcGFzcz0nJw0K

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.

_________________
Live as if you were to die tomorrow, Learn as if you were to live forever

The beauty of a living thing is not the atoms that go into it, but the way those atoms are put together!

2007/12/10 5:46:00
#11
Offline
Just can not stay away

Re: Auditing Code (security wise)

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


2007/12/10 8:37:44
#12
Offline
Home away from home

Re: Auditing Code (security wise)


_________________

Steve Twitter: @skenow Facebook: Steve Kenow


2007/12/10 12:09:41
#13
Offline
Home away from home

Re: Auditing Code (security wise)

Quote:


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



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


2007/12/10 12:27:19
#14
Offline
Home away from home

Re: Auditing Code (security wise)

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.


2007/12/10 12:52:53
#15
Offline
Home away from home

Re: Auditing Code (security wise)

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.

_________________
Live as if you were to die tomorrow, Learn as if you were to live forever

The beauty of a living thing is not the atoms that go into it, but the way those atoms are put together!

2007/12/10 13:16:50
#16
Offline
Home away from home

Re: Auditing Code (security wise)

Quote:

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.


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.

_________________
Marc-André Lanciault
Founder and CEO INBOX International inc.
Co-Founder ImpressCMS

2007/12/13 19:03:30
#17
Offline
Home away from home

Re: Auditing Code (security wise)

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

_________________

Steve Twitter: @skenow Facebook: Steve Kenow


2007/12/14 3:39:50
#18
Offline
Home away from home

Re: Auditing Code (security wise)

Quote:

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



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


2007/12/14 4:07:14
#19
Offline
Just can not stay away

Re: Auditing Code (security wise)

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

DQpmb3JlYWNoICggJF9QT1NUIGFzICRrID0+ICR2ICkgew0KICAgICR7JGt9ID0gJHY7DQp9DQo=
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.


2007/12/14 4:48:10
#20
Offline
Home away from home

Re: Auditing Code (security wise)

Absolutely agree.

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

Quote:

foreach ( $_POST as $k => $v ) {
${$k} = $v;
}


or
Quote:

foreach ( $_GET as $k => $v ) {
${$k} = $v;
}


and submit a bug request about this ?

ANything else we should be lookig for ?

_________________
Marc-André Lanciault
Founder and CEO INBOX International inc.
Co-Founder ImpressCMS

Reply New Topic extras
 Previous Topic   Next Topic
You can view topic.
You can start a new topic.
You can reply to posts.
You cannot edit your posts.
You cannot delete your posts.
You cannot add new polls.
You cannot vote in polls.
You cannot attach files to posts.
You can post without approval.