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

Ossfuzz corp v7 #5837

Closed
wants to merge 4 commits into from
Closed

Ossfuzz corp v7 #5837

wants to merge 4 commits into from

Conversation

catenacyber
Copy link
Contributor

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/4125

Describe changes:

  • Runs CI on public corpuses of fuzz targets for coverage and ASAN bugs
  • simple fuzz driver can run on flat directories (not only files)
  • fix bug of uninitialized memory in bytemath keyword parsing
  • fix bug by avoiding over recursion between DecodeEthernet and DecodeMPLS

Modifies #5811 by counting the total number of layers so that too much recursion can't happen anywhere (and adding a field to the structure Packet to do so)

@catenacyber catenacyber requested review from victorjulien and a team as code owners February 11, 2021 10:33
@catenacyber catenacyber mentioned this pull request Feb 11, 2021
@@ -38,6 +38,8 @@
#include "util-unittest.h"
#include "util-debug.h"

#define MAX_ETH_OFFSET 256
Copy link
Member

Choose a reason for hiding this comment

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

is this still used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, removing in next PR

@@ -887,6 +891,15 @@ void CaptureStatsSetup(ThreadVars *tv, CaptureStats *s);
((p)->action |= a)); \
} while (0)

#define PKT_MAX_DECODED_LAYERS 16

#define PACKET_INCREASE_CHECK_LAYERS(p) do { \
Copy link
Member

Choose a reason for hiding this comment

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

I think adding a macro to hides a return statement inside is not a great solution. I understand the desire to keep things tidy, but this is not the way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok :-)

@catenacyber catenacyber mentioned this pull request Feb 11, 2021
@catenacyber
Copy link
Contributor Author

Replaced by #5838

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants