Policy script interpreting continues after a const variable is later declared as a global variable.

[seth]

Running this...

const smtp_ports = { 25/tcp };
global smtp_ports = { 25/tcp, 587/tcp };

throws this error...

(smtp_ports): error, already defined

and the script continues being interpreted even though the value of the smtp_ports variable has been deleted. As far as I can tell, the smtp_ports variable is put back into a state as though it has been defined without a type (implicit or explicit). This problem is related to ticket #34.

[seth]

Digging into this more, it looks like script interpreting should also be aborted when a variable is defined without an explicit type or an assignment at the same time as the variable declaration.

function asfdasdf()
    {
    local test;
    }

That outputs the error "(test): error, no type given". Since that error seems to be thrown regardless of where the "local test;" code segment is (in an event handler or function) and it's always caught at script interpreting time, in my opinion it should shutdown Bro when it's encountered because further errors can occur later due to this.

I suppose my only question is, why doesn't the ID::Error method already shutdown Bro?

[robin]

Are you seeing a core dump here as well? Or does Bro just keep parsing the scripts but then aborts normally before starting to process packets? If it's the latter,
I'd say it's actually ok: Bro keeps parsing to potentially find more errors; it might run into further problems down the road because of the earlier error but that's hard to avoid. But in any case Bro should not crash.

[seth]

There is no core dump here. Looking closer now, it looks like Bro never even starts throwing events. Processing stops once all of the global/const script processing is finished. The problem really only seems to show up when the value that was incorrectly doubly initialized is used like in ticket #34. It seems to be that the variable doesn't have either of the values you attempted to set it to. The variable loses all value once the warning is thrown.

It doesn't appear to be catastrophic for Bro currently because event processing never begins, but it doesn't look good for new users to see Bro segfaulting when they've made a mistake. I suppose if this is something that would take much effort to fix, it makes sense to just skip it for now since it doesn't impair Bro at all.

[seth]

Thinking more... maybe this isn't a problem at all? Maybe the value being null is the right thing to do when the double initialization is done and the real problem is just with ticket #34. Leaving this behavior the way it is could possibly help to point out any more problems similar to ticket #34 too.