Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

End the scanner in a better way if the host list is malformed. #625

Merged
merged 3 commits into from
Nov 23, 2020

Conversation

jjnicola
Copy link
Member

What:
End the scanner in a better way if the host list is malformed and send a message to the client in case of invalid target list

Why:
If the target host list is not empty but malformed, the hosts struct is NULL. Then gvm_hosts_resolve() segfaults

How:
Start a scan with gvm-cli (gsa already checks for malformed target) wiht 192.168.0.1/32 as host target.

Checklist:

Send a message to the client in case of invalid target list
Copy link
Member

@bjoernricks bjoernricks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to include the invalid target list in the error message for the client too?

src/attack.c Outdated
unresolved = gvm_hosts_resolve (hosts);
if (hosts == NULL)
{
sprintf (buf, "Invalid target list: %s.", hostlist);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buffer is very small (only 96 bytes) and hostlist can get very big. Use snprinf or g_snprintf() and a bigger buffer if its likely that hostlist will exceed 96 bytes length. You can use strlen to get the needed lenght and check it agains the buffer size.
Do we even limit the lenght of a hoststring?

src/attack.c Outdated
{
sprintf (buf, "Invalid target list: %s.", hostlist);
connect_main_kb (&main_kb);
message_to_client (main_kb, buf, NULL, NULL, "ERRMSG");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

message_to_client has internal buffer with len 2048. Make sure that no buffer overflow can happen in this function too.

@jjnicola jjnicola marked this pull request as draft November 23, 2020 07:36
Copy link
Member

@ArnoStiefvater ArnoStiefvater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

g_strdup_printf is a good solution.

@jjnicola jjnicola merged commit 11c0009 into greenbone:openvas-20.08 Nov 23, 2020
@jjnicola jjnicola deleted the empty-host branch November 23, 2020 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants