detect/cip: add check for sscanf return-value - v5

[0xEniola]

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

Previous PR: #10418

Describe changes:

• Format specifier used on message for SCLogError re: sscanf [line 161] was wrong.

[victorjulien]

Can you show some examples of Suricata's output with malformed rules that display this error?

[0xEniola]

Can you show some examples of Suricata's output with malformed rules that display this error?

When I tested using unit tests, I found out with all the inputs I tried, the checks in the while case at line 113, caught all wrong user inputs before getting to the sscanf part, so there was no point where it got triggered.

[0xEniola]

Can you show some examples of Suricata's output with malformed rules that display this error?

Had to remove the other if and else if statements in the while statement, and left only the one with sscanf.

Test DetectCipServiceParseTest01                                  : Debug: time: offline time mode enabled [TimeModeSetOffline:util-time.c:108]
Debug: time: time set to 1708245819 sec, 436821 usec [TimeSet:util-time.c:133]
Error: detect-cipservice: incorrect input format; token xxmp should be uint8 [DetectCipServiceParse:detect-cipservice.c:116]
Debug: detect-cipservice: Returning pointer (nil) of type DetectENIP ... << [DetectCipServiceParse:detect-cipservice.c:150]
FAILED
==== TEST RESULTS ====
PASSED: 0
FAILED: 1
======================```

[0xEniola]

I feel I also need to change the error message to incorrect input format; token "???" can not be converted to uint8 instead.

I'm also wondering why sscanf was not made to be at the beginning of the while case to be sure the input can be converted to uint8 first before performing the other checks.

[jufajardini]

I feel I also need to change the error message to incorrect input format; token "???" can not be converted to uint8 instead.

Minor suggestion: could instead of can not

I'm also wondering why sscanf was not made to be at the beginning of the while case to be sure the input can be converted to uint8 first before performing the other checks.

I'm guessing that this might be to ensure the input is a list of integers comma-separated. But not sure.

[0xEniola]

I feel I also need to change the error message to incorrect input format; token "???" can not be converted to uint8 instead.

Minor suggestion: could instead of can not

I'm also wondering why sscanf was not made to be at the beginning of the while case to be sure the input can be converted to uint8 first before performing the other checks.

I'm guessing that this might be to ensure the input is a list of integers comma-separated. But not sure.

I feel I also need to change the error message to incorrect input format; token "???" can not be converted to uint8 instead.

Minor suggestion: could instead of can not

I'm also wondering why sscanf was not made to be at the beginning of the while case to be sure the input can be converted to uint8 first before performing the other checks.

I'm guessing that this might be to ensure the input is a list of integers comma-separated. But not sure.

Yes, could will be better. Thank you for the sugestion.

I tried comma seperated inputs, and it did not work though. So, I really do not know.

[jufajardini]

I feel I also need to change the error message to incorrect input format; token "???" can not be converted to uint8 instead.

Minor suggestion: could instead of can not

I'm also wondering why sscanf was not made to be at the beginning of the while case to be sure the input can be converted to uint8 first before performing the other checks.

I'm guessing that this might be to ensure the input is a list of integers comma-separated. But not sure.

I feel I also need to change the error message to incorrect input format; token "???" can not be converted to uint8 instead.

Minor suggestion: could instead of can not

I'm also wondering why sscanf was not made to be at the beginning of the while case to be sure the input can be converted to uint8 first before performing the other checks.

I'm guessing that this might be to ensure the input is a list of integers comma-separated. But not sure.

Yes, could will be better. Thank you for the sugestion.

I tried comma seperated inputs, and it did not work though. So, I really do not know.

Can you elaborate on how did you try that?

[0xEniola]

I feel I also need to change the error message to incorrect input format; token "???" can not be converted to uint8 instead.

Minor suggestion: could instead of can not

I'm also wondering why sscanf was not made to be at the beginning of the while case to be sure the input can be converted to uint8 first before performing the other checks.

I'm guessing that this might be to ensure the input is a list of integers comma-separated. But not sure.

I feel I also need to change the error message to incorrect input format; token "???" can not be converted to uint8 instead.

Minor suggestion: could instead of can not

I'm also wondering why sscanf was not made to be at the beginning of the while case to be sure the input can be converted to uint8 first before performing the other checks.

I'm guessing that this might be to ensure the input is a list of integers comma-separated. But not sure.

Yes, could will be better. Thank you for the sugestion. I tried comma seperated inputs, and it did not work though. So, I really do not know.

Can you elaborate on how did you try that?

I did it via the unittests, but I guess I did it wrong before; I tried it again now, and it works.

[catenacyber]

Stale PR Proposing to use https://github.com/OISF/suricata/pull/10901 which will fix it