sml_server or libsml crashing in "faulty" binary sml dump

[devZer0]

for a while now there is a "faulty" sml binary dump in https://github.com/devZer0/libsml-testing ( EMH_eHZ-IW8E2A5L0EK2P_crash.bin )

i changed sml_server.c to open fd=0 (which is stdin) and feeding sml_server with that, it's crashing. even with the most recent version from holger's repo.

from what i found so far is that entry->value->type seems to be an invalid pointer/reference, as the crash happens when accessing that.

cat /tmp/EMH_eHZ-IW8E2A5L0EK2P_crash.bin |./sml_server /dev/ttyUSB0 SML file (3 SML messages, 344 bytes) SML message 101 SML message 701 SML message 201 OBIS data 129-129:199.130.3255#EMH# 1-0:0.0.9255#06 45 4d 48 01 07 19 7c 24 56 # 1-0:1.8.0255#2795692.7#Wh 1-0:1.8.1255#2795692.7#Wh 1-0:1.8.2255#0.0#Wh 1-0:16.7.0255#136.7#W 129-129:199.130.5*255#8b 6a 0e 6e 12 f5 d9 80 f7 30 b6 bd 5e 19 41 83 4e b0 e4 3e 4a 63 23 d9 99 25 95 56 f5 e5 6e 04 04 98 c8 97 38 f0 f6 df f8 78 5b 04 5d 84 e0 d6 # Speicherzugriffsfehler

i think, whatever data enters libsml and being parsed there - it should not lead to any crash.

[r00t-]

with current volkszaehler/libsml master:

[/tmp/libsml]$ gdb ./examples/sml_server
Reading symbols from ./examples/sml_server...done.
(gdb) run - <EMH_eHZ-IW8E2A5L0EK2P_crash.bin 
Starting program: /tmp/libsml/examples/sml_server - <EMH_eHZ-IW8E2A5L0EK2P_crash.bin
SML file (3 SML messages, 344 bytes)
SML message  101
SML message  701
SML message  201
OBIS data
libsml: error: unknown type 0 in sml_value_to_double
libsml: error: unknown type 0 in sml_value_to_double
1-0:1.8.0*255#2795692.7#Wh
1-0:1.8.1*255#2795692.7#Wh
1-0:16.7.0*255#136.7#W
libsml: error: unknown type 0 in sml_value_to_double

Program received signal SIGSEGV, Segmentation fault.
0x0000000000401153 in transport_receiver (buffer=0x7fffffffc430 "\033\033\033\033\001\001\001\001v\a", buffer_len=360)
    at sml_server.c:104
104                                             switch (*entry->unit) {
(gdb) p *entry
$1 = {obj_name = 0x60cd80, status = 0x0, val_time = 0x0, unit = 0x0, scaler = 0x0, value = 0x60cdc0, value_signature = 0x0, next = 0x60ce00}
(gdb) 

[r00t-]

 44 typedef struct sml_list_entry {
 45     octet_string *obj_name;
 46     sml_status *status; // optional
 47     sml_time *val_time; // optional
 48     sml_unit *unit; // optional

this seems to indicate this first bug is in sml_server.c, relying on an optional field being present.

but there's more after that...

[r00t-]

Program received signal SIGSEGV, Segmentation fault.
0x0000000000402f4a in sml_value_to_double (value=0x0) at src/sml_value.c:113
113             switch (value->type) {

downright silly, the parser returns an entry with a null value and we pass that to _to_double...

i do not know if it's sensible for the parser to return such an entry.

[r00t-]

after fixing those two (both in sml_server.c), no more crashing. (instead the program hangs because it ignores EOF)

[r00t-]

does not seem worth forking the repo for this:

[/tmp/libsml]$ git diff
diff --git a/examples/sml_server.c b/examples/sml_server.c
index df29996..bbaabdd 100644
--- a/examples/sml_server.c
+++ b/examples/sml_server.c
@@ -37,6 +37,8 @@ int serial_port_open(const char* device) {
        struct termios config;
        memset(&config, 0, sizeof(config));

+    if (!strcmp(device,"-")) return 0; // read stdin when "-" is given for the device
+
 #ifdef O_NONBLOCK
        int fd = open(device, O_RDWR | O_NOCTTY | O_NONBLOCK);
 #else
@@ -90,6 +92,7 @@ void transport_receiver(unsigned char *buffer, size_t buffer_len) {
                        sml_get_list_response *body;
                        body = (sml_get_list_response *) message->message_body->data;
                        for (entry = body->val_list; entry != NULL; entry = entry->next) {
+                               if (!entry->value) continue; // do not crash on null entries
                                value = sml_value_to_double(entry->value);
                                if (value != 0) {
                                        int scaler = (entry->scaler) ? *entry->scaler : 1;
@@ -99,7 +102,7 @@ void transport_receiver(unsigned char *buffer, size_t buffer_len) {
                                                entry->obj_name->str[0], entry->obj_name->str[1],
                                                entry->obj_name->str[2], entry->obj_name->str[3],
                                                entry->obj_name->str[4], entry->obj_name->str[5], value);
-                                       switch (*entry->unit) {
+                                       if (entry->unit) switch (*entry->unit) {
                                        case 0x1B:
                                                printf("W");
                                                break;
@@ -129,15 +132,15 @@ int main(int argc, char *argv[]) {
        }

        // check for serial port
-       if (access(argv[1], F_OK) == -1) {
+       if (strcmp(argv[1],"-")&&access(argv[1], F_OK) == -1) {
                printf("Error: no such device (%s)\n", argv[1]);
                exit(2);
        }

        // open serial port
        int fd = serial_port_open(argv[1]);
-       if (!fd) {
-               printf("Error: can''t open device (%s)\n", argv[1]);
+       if (fd<0) {
+               printf("Error: can't open device (%s)\n", argv[1]);
                exit(3);
        }

[r00t-]

here is a PR after all: https://github.com/volkszaehler/libsml/issues/34

[r00t-]

i would say the libsml code is pretty stable otherwise:

[/tmp/libsml]$ bfr -p </dev/urandom | ./examples/sml_server -
[ ######################################## ][ 760.7k/s->5088.0k->757.6k/s (142292.0k) ]

(we might want to create an input with more valid sml frame headers)