Skip to content

Add length verification for preventing buffer overflow #11319

@7erryX

Description

@7erryX

Previous Patch

Commit 4e7894d addressed OSV-2020-2014 by checking if the value length is larger than the expected string string.

                else if (v->type == MSGPACK_OBJECT_NEGATIVE_INTEGER) {
                    val = temp
                    val_len = snprintf(temp, sizeof(temp) - 1,
                                       "%" PRId64, v->via.i64);
+                    /*
+                     * Check if the value length is larger than our string.
+                     * this is needed to avoid stack-based overflows.
+                     */
+                    if (val_len > sizeof(temp)) {
+                        return NULL;
+                    }
                }

Similar code

Some code snippets e.g. influxdb_format in plugins/out_influxdb/influxdb.c contains a similar logic(from line 177)

            else if (v->type == MSGPACK_OBJECT_POSITIVE_INTEGER) {
                val = tmp;
                if (ctx->use_influxdb_integer) {
                    val_len = snprintf(tmp, sizeof(tmp) - 1, "%" PRIu64 "i", v->via.u64);
                }
                else {
                    val_len = snprintf(tmp, sizeof(tmp) - 1, "%" PRIu64, v->via.u64);
                }
            }
            else if (v->type == MSGPACK_OBJECT_NEGATIVE_INTEGER) {
                val = tmp;
                if (ctx->use_influxdb_integer) {
                    val_len = snprintf(tmp, sizeof(tmp) - 1, "%" PRId64 "i", v->via.i64);
                }
                else {
                    val_len = snprintf(tmp, sizeof(tmp) - 1, "%" PRId64, v->via.i64);
                }
            }
            else if (v->type == MSGPACK_OBJECT_FLOAT || v->type == MSGPACK_OBJECT_FLOAT32) {
                val = tmp;
                val_len = snprintf(tmp, sizeof(tmp) - 1, "%f", v->via.f64);
            }

Full list

  • influxdb_format in plugins/out_influxdb/influxdb.c
  • msgpack_to_sd in plugins/out_syslog/syslog.c
  • msgpack_to_syslog in plugins/out_syslog/syslog.c
  • pack_format_line_value in plugins/out_loki/loki.c

Would it make sense to add similar checks as Commit 4e7894d did for preventing buffer overflow?

Apologies if I missed anything or caused inconvenience

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions