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

Fix repeated wodle modules #1128

Merged
merged 6 commits into from
Aug 22, 2018
Merged

Fix repeated wodle modules #1128

merged 6 commits into from
Aug 22, 2018

Conversation

cristgl
Copy link
Contributor

@cristgl cristgl commented Aug 20, 2018

This PR fixes the issue #1057.
Added a new field to wodle modules named tag, consisting of the 'command' word plus the command tag in case of the Command wodle, in other case, it shows the name of the module.

@cristgl cristgl requested a review from vikman90 August 20, 2018 12:36
@cristgl cristgl added this to Needs review in Wazuh 3.5.1 via automation Aug 20, 2018
Copy link
Member

@vikman90 vikman90 left a comment

Choose a reason for hiding this comment

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

Hi @cristgl,

we miss two things:

  • Memory destructor for the new tag.
  • Changelog entry.

Thanks!

command_tag_length = strlen(WM_COMMAND_CONTEXT.name) + 2;
command_tag = malloc(sizeof(char) * command_tag_length);
snprintf(command_tag, command_tag_length, "%s", WM_COMMAND_CONTEXT.name);
os_strdup(command_tag, nodes[i]->content);
Copy link
Member

Choose a reason for hiding this comment

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

os_strdup sets from left to right. In this case, it would be overwriting the content of the XML structure and producing a memory leak.

Did you mean...?

os_strdup(command_tag, command->tag);

snprintf(command_tag, command_tag_length, "%s:%s", WM_COMMAND_CONTEXT.name, command->tag);
}

os_strdup(command_tag, module->tag);
Copy link
Member

Choose a reason for hiding this comment

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

This sentence duplicates the string at command_tag, but this variable is never freed. This produces a memory leak.

if (i->context->name == j->context->name) {
mdebug1("Deleting repeated module '%s'.", j->context->name);
if(i->tag && j->tag){
if(!strcmp(i->tag,"command") || !strcmp(j->tag,"command")){
Copy link
Member

Choose a reason for hiding this comment

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

This should not be mandatory at this point. We should warn about it in the configuration reader.

@vikman90 vikman90 self-assigned this Aug 20, 2018
@vikman90 vikman90 added the type/bug Something isn't working label Aug 20, 2018
@vikman90 vikman90 moved this from Needs review to In progress in Wazuh 3.5.1 Aug 20, 2018
@vikman90 vikman90 moved this from In progress to Needs review in Wazuh 3.5.1 Aug 20, 2018
@crolopez crolopez moved this from Needs review to Reviewer approved in Wazuh 3.5.1 Aug 22, 2018
@crolopez crolopez self-assigned this Aug 22, 2018
@vikman90 vikman90 merged commit e0479e7 into 3.5 Aug 22, 2018
Wazuh 3.5.1 automation moved this from Reviewer approved to Done Aug 22, 2018
@vikman90 vikman90 deleted the fix-repeated-wodle branch August 22, 2018 11:09
@vikman90
Copy link
Member

GJ @cristgl !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something isn't working
Projects
No open projects
Wazuh 3.5.1
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants