yaml: Add check of allocation for node object v2

[jtstrs]

Fix potential dereference of nullptr in case
of unsuccessful allocation of memory for
list node

Bug: #7270

Contribution style:

• I have read the contributing guide lines at
  https://docs.suricata.io/en/latest/devguide/contributing/contribution-process.html

Our Contribution agreements:

• I have signed the Open Information Security Foundation contribution agreement at
  https://suricata.io/about/contribution-agreement/ (note: this is only required once)

Changes (if applicable):

• I have updated the User Guide (in doc/userguide/) to reflect the changes made
• I have updated the JSON schema (in etc/schema.json) to reflect all logging changes
  (including schema descriptions)
• I have created a ticket at
  https://redmine.openinfosecfoundation.org/projects/suricata/issues

Link to ticket: https://redmine.openinfosecfoundation.org/issues/7270
Describe changes:

• Add check of node pointers after allocation of memory;
• If memory allocated successfully, behavior do not change;
• If allocation fail, goto fail marker;

Follows #11822 with transforming commit message to proper state

[jtstrs]

@jasonish, @catenacyber

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.56%. Comparing base (7ab8334) to head (ad68d0a).
Report is 24 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11847      +/-   ##
==========================================
- Coverage   82.58%   82.56%   -0.02%     
==========================================
  Files         914      912       -2     
  Lines      249500   249353     -147     
==========================================
- Hits       206045   205887     -158     
- Misses      43455    43466      +11     

Flag	Coverage Δ
fuzzcorpus	60.47% <50.00%> (+0.04%)	⬆️
livemode	18.76% <50.00%> (+0.03%)	⬆️
pcap	44.09% <50.00%> (+0.01%)	⬆️
suricata-verify	62.03% <50.00%> (+<0.01%)	⬆️
unittests	58.93% <50.00%> (-0.04%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

[jufajardini]

wrt the unit test checks mentioned by @catenacyber , I'm assuming they're the ones in conf.c, such as parent and child in this test and https://github.com/OISF/suricata/blob/master/src/conf.c#L1117-L1125 and the following ConfNodeLookupChildValueTest.

Should these be addressed as part of this work?

[inashivb]

wrt the unit test checks mentioned by @catenacyber , I'm assuming they're the ones in conf.c, such as parent and child in this test and https://github.com/OISF/suricata/blob/master/src/conf.c#L1117-L1125 and the following ConfNodeLookupChildValueTest.

Should these be addressed as part of this work?

From what I see, unittests are too dirty as they are and we're ok w that as long as its on the fail path. Running unittests w ASAN enabled will give you a long list of issues.
Sure it'll be good to clean them all up. Not sure if this work should be stalled bc of that..

[catenacyber]

Thanks for the work.

CI : ok
Commits segmentation : ok
Commit messages : ok
Git ID set : ok
CLA : ❓ I do not have access
Doc update : not needed
Redmine ticket : ok
Rustfmt : no rust
Tests : ❓ would you know how to test for this ? What was the static analysis tool ? Was it the only failed allocation missing test ?
Dependencies added: none
Code : good

[jtstrs]

@catenacyber , unfortunately I'm couldn't tell you a name of static analyzer. Also I wasn't available to reproduce this execution path in runtime. Static analyzer triggered only on fact that return value of function wasn't checked on null, but there is cases when function return nullptr(only one, when we couldn't allocate of memory)

[catenacyber]

Looks like github/codeql#16524

[jtstrs]

Yep, exactly

[catenacyber]

Yep, exactly

For the record, running locally this CodeQL query, using CodeQL suricata database downloaded from GitHub, I do not find this bug (even if it looks like I should) (and it finds other stuff related to lua)

[jtstrs]

Sorry, probably my answer wasn't clear enough, I mean that static analyzer, which I've used have same type checker

[victorjulien]

Merged in #11858, thanks!