Add null check for pointer-based function arguments

[timwoj]

Fixes #33

This adds a simple check for bif function arguments that map to pointer types. Unfortunately due to the generic-ness of the types defined in include/bif_type.def, the only real way to determine if it's a pointer is to check if the type string ends with *.

[timwoj]

Hmm, this fix breaks the zeek core.expired-conn-weird btest.

[  0%] core.expired-conn-weird ... failed
  % 'btest-diff out' failed unexpectedly (exit code 1)
  % cat .diag
  == File ===============================
  == Diff ===============================
  --- /dev/fd/63	2025-03-06 20:51:40
  +++ /dev/fd/62	2025-03-06 20:51:40
  @@ -1,2 +0,0 @@
  -expired_conn_weird, test!, [orig_h=192.168.1.200, orig_p=49206/tcp, resp_h=192.168.1.150, resp_p=3389/tcp, proto=6], CHhAvVGS1DHFjwGM9, test2
  -expired_conn_weird, test!, [orig_h=192.168.1.200, orig_p=49206/tcp, resp_h=192.168.1.150, resp_p=3389/tcp, proto=6], CHhAvVGS1DHFjwGM9, test2
  =======================================

  % cat .stderr
  1297551079.965640 error in /Users/tim/Desktop/projects/zeek-master/testing/btest/.tmp/core.expired-conn-weird/expired-conn-weird.zeek, line 8: Value for argument c is invalid/null (Reporter::conn_weird(test!, c, test2, ))
  1297551079.965640 error in /Users/tim/Desktop/projects/zeek-master/testing/btest/.tmp/core.expired-conn-weird/expired-conn-weird.zeek, line 9: Value for argument c is invalid/null (Reporter::conn_weird(test!, c, test2, ))
  1297551079.965640 error in /Users/tim/Desktop/projects/zeek-master/testing/btest/.tmp/core.expired-conn-weird/expired-conn-weird.zeek, line 10: Value for argument c is invalid/null (Reporter::conn_weird(test!, c, test2, ))
  1297551079.965640 error in /Users/tim/Desktop/projects/zeek-master/testing/btest/.tmp/core.expired-conn-weird/expired-conn-weird.zeek, line 11: Value for argument c is invalid/null (Reporter::conn_weird(test!, c, test2, ))

1 of 1 test failed

[awelzel]

Would that be better than before line 60, where we already would access a nullptr - runtime_type_check is only true for variable args, so maybe the combination doesn't really happen right now.

[timwoj]

There are definitely places where the second one can get generated without the first one. I added a null check on the first one plus a flag for whether it got generated to avoid generating it twice.

[timwoj]

The btest failure is actually interesting, and might negate this approach to fixing this bug entirely. The Report::conn_weird BIF with this change generates the following:

#line 125 "/Users/tim/Desktop/projects/zeek-master/src/reporter.bif"
zeek::ValPtr zeek::BifFunc::Reporter::conn_weird_bif(zeek::detail::Frame* frame, const zeek::Args* BiF_ARGS)
	
#line 126 "/Users/tim/Desktop/projects/zeek-master/src/reporter.bif"
{
	if ( BiF_ARGS->size() != 4 )
		{
		zeek::emit_builtin_error(zeek::util::fmt("Reporter::conn_weird() takes exactly 4 argument(s), got %lu", BiF_ARGS->size()));
		return nullptr;
		}
	zeek::StringVal* name = (zeek::StringVal*) (BiF_ARGS->at(0).get()->AsStringVal());
	 if ( name == nullptr ) {
		zeek::emit_builtin_error("Value for argument name is invalid/null");
		return nullptr;
	}
	zeek::Connection* c = (zeek::Connection*) (BiF_ARGS->at(1).get()->AsRecordVal()->GetOrigin());
	 if ( c == nullptr ) {
		zeek::emit_builtin_error("Value for argument c is invalid/null");
		return nullptr;
	}
	zeek::StringVal* addl = (zeek::StringVal*) (BiF_ARGS->at(2).get()->AsStringVal());
	 if ( addl == nullptr ) {
		zeek::emit_builtin_error("Value for argument addl is invalid/null");
		return nullptr;
	}
	zeek::StringVal* source = (zeek::StringVal*) (BiF_ARGS->at(3).get()->AsStringVal());
	 if ( source == nullptr ) {
		zeek::emit_builtin_error("Value for argument source is invalid/null");
		return nullptr;
	}

#line 126 "/Users/tim/Desktop/projects/zeek-master/src/reporter.bif"

	if ( c )
		reporter->Weird(c, name->CheckString(), addl->CheckString(), source->CheckString());
	else
		{
		auto connection_record = (*BiF_ARGS)[1]->AsRecordVal();
		auto conn_id_val = connection_record->GetField<RecordVal>("id");
		auto uid_val = connection_record->GetField<StringVal>("uid");
		reporter->Weird(conn_id_val, uid_val,
		                name->CheckString(), addl->CheckString(), source->CheckString());
		}

Note that it already has a check for an invalid connection at the bottom and reports a weird for that. The null checks that were added by this PR's change supersede that check, return an error, and cause the function to return a nullptr when it wouldn't have before.