Hi Andres.

On Sat, Jun 2, 2012 at 11:19 PM, Andres Riancho <andres.rian...@gmail.com>wrote:

> List,
>
>    During PHDays we had a really good idea with Miroslav: "I review
> sqlmap's code and send you some comments about it, and Miroslav will
> review some w3af code and do the same". So, while I had some spare
> minutes at the airport I performed some initial review:
>
> * I like the idea of using psyco, what's your experience with it? Do
> you guys recommend it?
>
It was a good thing till it lasted (Python <= v2.6) but as it's official
page says "12 March 2012  Psyco is unmaintained and dead" (
http://psyco.sourceforge.net/), so it's advised to not use it.

>
> * Liked the concept in "def smokeTest():", which sounds interesting to
> have also in w3af
>
Ok

>
> * lib/core/testing.py : shouldn't most/all of this be migrated to
> unit-tests and run using "nosetests" or some other tool like that?
>
In majority of cases it's impossible to use any of those python-based
testing tools if you need to run a testing program as an standalone
executable (not as a same program same module). We need to run it as a
standalone against testing environment (xml/livetests.xml) and parse the
output to see if it went ok. Look into this 'testing.py' as our way how to
deal with that problem (without using any 3rd party tools).

>
> * As Miroslav mentioned, we're using the same keepalive.py module,
> I'll have to run a diff between w3af's and sqlmap's and see what we
> changed; since we both made modifications to "make it work".
>
Ok

>
> * Using rangehandler.py is a great idea for speeding up (A LOT) the
> extraction of information, it seems that you guys add it to the
> urlopener but don't use it?
>
We use it in --null-connection (and implicitly in -o) for boolean-based
blind cases. If you take a look into lib/core/option.py you'll see that in
def __urllib2Opener() it's installed among other handlers. Also, if you
take a look into rangehandler.py you'll see that it's sole purpose is to
properly handle 206 and 416 HTTP codes related to those range-cases. Grep
for "kb.nullConnection" and you'll see how "Range" (or we call it "null
connection") method is used (extremely fast if available for boolean-based
blind cases)

>
> * Could you please explain me the first part of this if? "if
> conf.hostname in ('localhost', '127.0.0.1') or conf.ignoreProxy:" does
> it really make sense? Aren't you ignoring the user's wish?
>
Python, as you know, uses an automatic extraction of proxy information from
current environment (e.g. http_proxy env variable). Now, in 99% of cases
you don't want your automatic proxy settings to affect your access to the
localhost (be real, in most of browser settings first thing on the ignore
proxy list are localhost/127.0.0.1). That way we are just dealing with
major number of users who would complain about accessing localhost web
server and not reaching it (because corporate proxy settings were used
automatically)

>
> * heh, I also use gprof2dot for profiling, but instead of having it
> inside w3af, I simply call it from the command line and have it
> generate a PNG. Note, where is "start()" defined for this line?
> cProfile.run("start()", profileOutputFile)
>
that start() is defined inside the lib/controller/controller.py (it
represents the first sqlmap call that starts setting up everything and runs
the tool's functionality). string "start()" represents an eval-like python
call that will be called from the main() perspective. if you take a look
into the main() you'll see that start() is reachable from there.

>
> * Read this comment:
>    """
>    # Set kb.partRun in case "common prediction" feature (a.k.a. "good
>    # samaritan") is used
>    """
>
> Good samaritan was a feature I added many years ago to w3af's sqlmap,
> and the name came from the idea that the user could help the blind sql
> injection process by completing the word that was being extracted.
> Example: "If sqlmap extracted -hello w- the user could type -orld- in
> the console and have it checked with a SELECT statement". According to
> the pieces of code I was able to find, that was replaced by a more
> automatic idea where a file feeds common strings to the process,
> correct? The idea sounds good, but maybe users still want to
> contribute to the process?
>
I am interested how you managed to get the user's input while outputting
the results in the same time? We've always had a problem where you have to
provide an user with that "raw_input" functionality and in the same time do
the output. Problem with Python is that it doesn't give you something like
"Keyboard Hooking" that would easify this all problem.

>
> * dataToStdout() is a handy function, but I think that you should
> consider migrating to something more generic like python's logging
> module. If in the future you want to provide options to storing the
> data in a file, or similar, it might come handy. In w3af we have the
> outputManag
>
We are using both logging module and dataToStdout. dataToStdout can be
called from anywhere at any time and it will always output (in thread safe
manner) just the thing you've given to it. Logger as the other approach
does the output of the given text in an line manner (!) and that would be a
very bad thing especially when you want to output character by character.
Also, logger outputs everything in a message like structure (prepending
e.g. [CRITICAL]) and in lots of cases we don't want that. So, those two are
synergetic in a way and we need them both for a proper sqlmap run.

>
> - From our talks I understood that sqlmap used multiprocessing for
> cracking hashes (or something like that) but I can't find any
> reference to the multiprocessing module in the latest version. Could
> you point me in the right direction so I can analyze that code?
>
lib/utils/hash.py

>
> - Not sure how usable it is for you guys, but in some cases the
> charset is set in a meta tag; you're ignoring that here:
>    if contentType and (contentType.find('charset=') != -1):
>        charset = checkCharEncoding(contentType.split('charset=')[-1])
>
>        if charset:
>            page = getUnicode(page, charset)
>
I am not sure if you are using the latest revision from our repository (go
to www.sqlmap.org for proper "svn checkout" line).

Those few lines go like this (in latest v1.0-dev):
...
        if contentType and (contentType.find('charset=') != -1):
            httpCharset =
checkCharEncoding(contentType.split('charset=')[-1])

        metaCharset =
checkCharEncoding(extractRegexResult(META_CHARSET_REGEX, page, re.DOTALL |
re.IGNORECASE))
...
We are not ignoring the metaCharset. We are using them both (while
httpCharset has the higher priority) in following code.

>
>  See w3af's httpResponse.py for an example on how we're doing it.
>
> - Not thread safe?
>
>        if conf.delay is not None and isinstance(conf.delay, (int,
> float)) and conf.delay > 0:
>            time.sleep(conf.delay)
>
But those few lines are IMHO irrelevant for any "thread-safe" manner.
Thread safe means that you have to be careful to prevent situations where
something critical could be changed in the same time as other thread is
reading it (or vice versa/similar) and this is really something of no
interest in that field.

If you though that time.sleep() blocks the whole process, that's not the
case. It blocks only the current thread (
http://stackoverflow.com/questions/92928/time-sleep-sleeps-thread-or-process),
so nothing to be worried in this field too.

>
>  Maybe move the "kb.locks.reqLock.acquire()" some lines before?
>
No need

>
> - Doesn't this kill the keepalive.py handler? Should try to capture
> packets.
>
>            if not req.has_header("Connection"):
>                requestHeaders += "\nConnection: close"
>
Those requestHeaders is just a "log entry" and it doesn't kill the
"keep-alive" functionality. This was just a dirty hack where everything has
been declared (in log/traffic files) as connection close (to appear like to
the end user) as in that point you can't know if something is really
keep-alive or not (you can take a look into the header content and you
won't see a thing - it's handled by a keepalive handler in a low-level
manner). Now, we could do some dirty hacks to signal from keepalive handler
if something is really Keep-Alive (I am saying that because there are lots
of cases where Keep-Alive is just not possible or dropped in some point)
and properly do the logging stuff but this is of low priority this moment.

>
> I know that many of these are questions, but I hope they trigger some
> good ideas :)
>
Thank you for your observations :)

>
> PS: I only used 2h for reading code. 2h left.
>
:)

I'll try to do mine this week. Prepare yourself.

>
> Regards,

--
> Andrés Riancho
> Project Leader at w3af - http://w3af.org/
> Web Application Attack and Audit Framework
> Twitter: @w3af
> GPG: 0x93C344F3
>
> Kind regards,
Miroslav Stampar

>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> sqlmap-users mailing list
> sqlmap-users@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/sqlmap-users
>



-- 
Miroslav Stampar
http://about.me/stamparm
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
sqlmap-users mailing list
sqlmap-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sqlmap-users

Reply via email to