From 371010594592b753a2c467b5d2b97f39a9f27770 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Mon, 20 May 2019 16:48:20 +0200 Subject: [PATCH] MINOR: tools: provide a may_access() function and make dump_hex() use it It's a bit too easy to crash by accident when using dump_hex() on any area. Let's have a function to check if the memory may safely be read first. This one abuses the stat() syscall checking if it returns EFAULT or not, in which case it means we're not allowed to read from there. In other situations it may return other codes or even a success if the area pointed to by the file exists. It's important not to abuse it though and as such it's tested only once per output line. --- include/common/standard.h | 3 ++- src/debug.c | 2 +- src/standard.c | 54 ++++++++++++++++++++++++++++++--------- 3 files changed, 45 insertions(+), 14 deletions(-) diff --git a/include/common/standard.h b/include/common/standard.h index 6fb1c9442..0bea022b9 100644 --- a/include/common/standard.h +++ b/include/common/standard.h @@ -1423,7 +1423,8 @@ int dump_text(struct buffer *out, const char *buf, int bsize); int dump_binary(struct buffer *out, const char *buf, int bsize); int dump_text_line(struct buffer *out, const char *buf, int bsize, int len, int *line, int ptr); -void dump_hex(struct buffer *out, const char *pfx, const void *buf, int len); +void dump_hex(struct buffer *out, const char *pfx, const void *buf, int len, int unsafe); +int may_access(const void *ptr); /* same as realloc() except that ptr is also freed upon failure */ static inline void *my_realloc2(void *ptr, size_t size) diff --git a/src/debug.c b/src/debug.c index 91bf3a30d..b4fcb6539 100644 --- a/src/debug.c +++ b/src/debug.c @@ -325,7 +325,7 @@ static int debug_parse_cli_hex(char **args, char *payload, struct appctx *appctx len = ((start + 128) & -16) - start; chunk_reset(&trash); - dump_hex(&trash, " ", (const void *)start, len); + dump_hex(&trash, " ", (const void *)start, len, 1); trash.area[trash.data] = 0; appctx->ctx.cli.severity = LOG_INFO; appctx->ctx.cli.msg = trash.area; diff --git a/src/standard.c b/src/standard.c index ffabccb5e..1c2e5099c 100644 --- a/src/standard.c +++ b/src/standard.c @@ -20,6 +20,8 @@ #include #include #include +#include +#include #include #include #include @@ -4009,6 +4011,26 @@ fail_wl: return 0; } +/* indicates if a memory location may safely be read or not. The trick consists + * in performing a harmless syscall using this location as an input and letting + * the operating system report whether it's OK or not. For this we have the + * stat() syscall, which will return EFAULT when the memory location supposed + * to contain the file name is not readable. If it is readable it will then + * either return 0 if the area contains an existing file name, or -1 with + * another code. This must not be abused, and some audit systems might detect + * this as abnormal activity. It's used only for unsafe dumps. + */ +int may_access(const void *ptr) +{ + struct stat buf; + + if (stat(ptr, &buf) == 0) + return 1; + if (errno == EFAULT) + return 0; + return 1; +} + /* print a string of text buffer to . The format is : * Non-printable chars \t, \n, \r and \e are * encoded in C format. * Other non-printable chars are encoded "\xHH". Space, '\', and '=' are also escaped. @@ -4081,9 +4103,10 @@ int dump_binary(struct buffer *out, const char *buf, int bsize) * The output will not wrap pas the buffer's end so it is more optimal if the * caller makes sure the buffer is aligned first. A trailing zero will always * be appended (and not counted) if there is room for it. The caller must make - * sure that the area is dumpable first. + * sure that the area is dumpable first. If is non-null, the memory + * locations are checked first for being readable. */ -void dump_hex(struct buffer *out, const char *pfx, const void *buf, int len) +void dump_hex(struct buffer *out, const char *pfx, const void *buf, int len, int unsafe) { const unsigned char *d = buf; int i, j, start; @@ -4094,25 +4117,32 @@ void dump_hex(struct buffer *out, const char *pfx, const void *buf, int len) for (i = 0; i < start + len; i += 16) { chunk_appendf(out, (sizeof(void *) == 4) ? "%s%8p: " : "%s%16p: ", pfx, d + i); + // 0: unchecked, 1: checked safe, 2: danger + unsafe = !!unsafe; + if (unsafe && !may_access(d + i)) + unsafe = 2; + for (j = 0; j < 16; j++) { - if ((i + j >= start) && (i + j < start + len)) - chunk_appendf(out, "%02x ", d[i + j]); - else + if ((i + j < start) || (i + j >= start + len)) chunk_strcat(out, "'' "); + else if (unsafe > 1) + chunk_strcat(out, "** "); + else + chunk_appendf(out, "%02x ", d[i + j]); if (j == 7) chunk_strcat(out, "- "); } chunk_strcat(out, " "); for (j = 0; j < 16; j++) { - if ((i + j >= start) && (i + j < start + len)) { - if (isprint(d[i + j])) - chunk_appendf(out, "%c", d[i + j]); - else - chunk_strcat(out, "."); - } - else + if ((i + j < start) || (i + j >= start + len)) chunk_strcat(out, "'"); + else if (unsafe > 1) + chunk_strcat(out, "*"); + else if (isprint(d[i + j])) + chunk_appendf(out, "%c", d[i + j]); + else + chunk_strcat(out, "."); } chunk_strcat(out, "\n"); }