[pkg] brutespray - review

[Stéphane Neveu]

Hi Lukas,

2017-07-22 20:24 GMT+02:00 Lukas Schwaighofer <lukas at schwaighofer.name>:
> Hi,
>
> On Sat, 22 Jul 2017 09:03:17 +0200
> Stéphane Neveu <stefneveu at gmail.com> wrote:
>
>> Thank you, it's updated. Tell me if it looks better now :)
>
> Looks good to me; if you want to make the commands listing more
> beautiful, add a colon after the first line if each bullet point, i.e.
> (diff style)
>
> -  * `-f` FILE, `--file` FILE
> +  * `-f` FILE, `--file` FILE:
>      GNMAP or XML file to parse
>
> -  * `-s` SERVICE, `--service` SERVICE
> +  * `-s` SERVICE, `--service` SERVICE:
>      Specify service to attack
>
> and so on. From ronn-format(7), DEFINITIONS LIST section:
>
>        The definition list syntax is compatible with markdown´s
>        unordered list syntax but requires that the first line of each
>        list item be terminated with a colon
>
> But that's really a nit-pick, otherwise everything looks fine to me.
>

Great, ok I'll add those colons to make it look better.

>
> However I read a bit of the python code and brutespray.py does some
> things which will lead to unexpected results (e.g. it will basically
> perform the equivalent of `rm tmp/*` on each startup, which is
> something quite unexpected to do for a program that I start.
>
> Actually, I think that before we can improve on that behavior
> (preferably by submitting a pull request upstream patching brutespray
> to use a proper temporary directory), I think the program is unsuitable
> for Debian.  (I feel a bit personally affected by this as I usually keep
> a ~/tmp dir with some stuff that I am temporarily working on, so using
> this script could cause some things I'm working on to be lost.)
>
> Maybe ask a DD here for advise? I also CCd Sophie, maybe she has an
> advise or opinion as well.
>

    if not os.path.exists("tmp/"):
        os.mkdir("tmp/")
    tmppath = "tmp/"
    filelist = os.listdir(tmppath)
    for filename in filelist:
        os.remove(tmppath+filename)

Yes, that not really clean, I have to agree. No hope to fix that with
simple patch that replaces tmp/ by /tmp/brutespray ?
Nevertheless, I agree with you, it would be sane to submit a patch upstream.


>> > PS: I just noticed that we also uploaded curvedns with a priority
>> >     standard, that should also be corrected to optional or extra
>> > with a subsequent upload.
>>
>> Yes, I also need to update Vcs-* as I told you yesterday.
>
> Good that you noticed and fixed it in git!
>
Regards
Stephane