diff --git a/ssh/auth-options.c b/ssh/auth-options.c index 40876a1..67c154c 100644 --- a/ssh/auth-options.c +++ b/ssh/auth-options.c @@ -421,7 +421,7 @@ bad_option: #define OPTIONS_CRITICAL 1 #define OPTIONS_EXTENSIONS 2 static int -parse_option_list(const struct sshbuf *oblob, struct passwd *pw, +parse_option_list(struct sshbuf *oblob, struct passwd *pw, u_int which, int crit, int *cert_no_port_forwarding_flag, int *cert_no_agent_forwarding_flag, diff --git a/ssh/ssh-keygen.c b/ssh/ssh-keygen.c index a6c147e..893cfaf 100644 --- a/ssh/ssh-keygen.c +++ b/ssh/ssh-keygen.c @@ -1816,18 +1816,17 @@ add_cert_option(char *opt) } static void -show_options(const struct sshbuf *optbuf, int v00, int in_critical) +show_options(struct sshbuf *optbuf, int v00, int in_critical) { char *name, *data; - struct sshbuf *options, *option; + struct sshbuf *options, *option = NULL; int r; if ((options = sshbuf_fromb(optbuf)) == NULL) fatal("%s: sshbuf_new failed", __func__); - - if ((option = sshbuf_new()) == NULL) - fatal("%s: sshbuf_new failed", __func__); while (sshbuf_len(options) != 0) { + sshbuf_free(option); + option = NULL; if ((r = sshbuf_get_cstring(options, &name, NULL)) != 0 || (r = sshbuf_get_stringb(options, option)) != 0) fatal("%s: buffer error: %s", __func__, ssh_err(r)); diff --git a/ssh/sshbuf-getput-basic.c b/ssh/sshbuf-getput-basic.c index 1c40e55..d8bc847 100644 --- a/ssh/sshbuf-getput-basic.c +++ b/ssh/sshbuf-getput-basic.c @@ -368,8 +368,9 @@ sshbuf_froms(struct sshbuf *buf, struct sshbuf **bufp) return r; if ((ret = sshbuf_from(p, len)) == NULL) return SSH_ERR_ALLOC_FAIL; - if ((r = sshbuf_consume(buf, len + 4)) != 0) { /* Shouldn't happen */ - free(ret); + if ((r = sshbuf_consume(buf, len + 4)) != 0 || /* Shouldn't happen */ + (r = sshbuf_set_parent(ret, buf)) != 0) { + sshbuf_free(ret); return r; } *bufp = ret; diff --git a/ssh/sshbuf.c b/ssh/sshbuf.c index 7f34110..cd0938b 100644 --- a/ssh/sshbuf.c +++ b/ssh/sshbuf.c @@ -37,6 +37,8 @@ struct sshbuf { size_t max_size; /* Maximum size of buffer */ size_t alloc; /* Total bytes allocated to buf->d */ int readonly; /* Refers to external, const data */ + u_int refcount; /* Tracks self and number of child buffers */ + struct sshbuf *parent; /* If child, pointer to parent */ }; static inline int @@ -45,6 +47,7 @@ sshbuf_check_sanity(const struct sshbuf *buf) SSHBUF_TELL("sanity"); if (__predict_false(buf == NULL || (!buf->readonly && buf->d != buf->cd) || + buf->refcount < 1 || buf->refcount > SSHBUF_REFS_MAX || buf->cd == NULL || buf->max_size > SSHBUF_SIZE_MAX || buf->alloc > buf->max_size || @@ -62,7 +65,7 @@ sshbuf_maybe_pack(struct sshbuf *buf, int force) { SSHBUF_DBG(("force %d", force)); SSHBUF_TELL("pre-pack"); - if (buf->readonly) + if (buf->readonly || buf->refcount > 1) return; if (force || (buf->off >= SSHBUF_PACK_MIN && buf->off >= buf->size / 2)) { @@ -83,6 +86,8 @@ sshbuf_new(void) ret->alloc = SSHBUF_SIZE_INIT; ret->max_size = SSHBUF_SIZE_MAX; ret->readonly = 0; + ret->refcount = 1; + ret->parent = NULL; if ((ret->cd = ret->d = calloc(1, ret->alloc)) == NULL) { free(ret); return NULL; @@ -100,21 +105,39 @@ sshbuf_from(const void *blob, size_t len) return NULL; ret->alloc = ret->size = ret->max_size = len; ret->readonly = 1; + ret->refcount = 1; + ret->parent = NULL; ret->cd = blob; ret->d = NULL; return ret; } -struct sshbuf * -sshbuf_fromb(const struct sshbuf *buf) +int +sshbuf_set_parent(struct sshbuf *child, struct sshbuf *parent) { - /* - * XXX - we could detect modification of the parent buffer by - * registering a linkage between the two. E.g. have a sshbuf->parent - * pointer and a sshbuf->nchildren counter. Possibly overkill for - * regular use, but perhaps a good idea for diagnostics? - */ - return sshbuf_from(sshbuf_ptr(buf), sshbuf_len(buf)); + int r; + + if ((r = sshbuf_check_sanity(child)) != 0 || + (r = sshbuf_check_sanity(parent)) != 0) + return r; + child->parent = parent; + child->parent->refcount++; + return 0; +} + +struct sshbuf * +sshbuf_fromb(struct sshbuf *buf) +{ + struct sshbuf *ret; + + if (sshbuf_check_sanity(buf) != 0) + return NULL; + ret = sshbuf_from(sshbuf_ptr(buf), sshbuf_len(buf)); + if (sshbuf_set_parent(ret, buf) != 0) { + sshbuf_free(ret); + return NULL; + } + return ret; } void @@ -128,7 +151,23 @@ sshbuf_free(struct sshbuf *buf) * have been passed to us and continuing to scribble over memory would * be bad. */ - if (sshbuf_check_sanity(buf) == 0) + if (sshbuf_check_sanity(buf) != 0) + return; + /* + * If we are a child, the free our parent to decrement its reference + * count and possibly free it. + */ + if (buf->parent != NULL) { + sshbuf_free(buf->parent); + buf->parent = NULL; + } + /* + * If we are a parent with still-extant children, then don't free just + * yet. The last child's call to sshbuf_free should decrement our + * refcount to 0 and trigger the actual free. + */ + buf->refcount--; + if (buf->refcount > 0) return; if (!buf->readonly) { bzero(buf->d, buf->alloc); @@ -143,7 +182,7 @@ sshbuf_reset(struct sshbuf *buf) { u_char *d; - if (buf->readonly) { + if (buf->readonly || buf->refcount > 1) { /* Nonsensical. Just make buffer appear empty */ buf->off = buf->size; return; @@ -171,6 +210,18 @@ sshbuf_alloc(const struct sshbuf *buf) return buf->alloc; } +const struct sshbuf * +sshbuf_parent(const struct sshbuf *buf) +{ + return buf->parent; +} + +u_int +sshbuf_refcount(const struct sshbuf *buf) +{ + return buf->refcount; +} + int sshbuf_set_max_size(struct sshbuf *buf, size_t max_size) { @@ -179,11 +230,11 @@ sshbuf_set_max_size(struct sshbuf *buf, size_t max_size) int r; SSHBUF_DBG(("set max buf = %p len = %zu", buf, max_size)); - if ((r = sshbuf_check_sanity(buf)) < 0) + if ((r = sshbuf_check_sanity(buf)) != 0) return r; if (max_size == buf->max_size) return 0; - if (buf->readonly) + if (buf->readonly || buf->refcount > 1) return SSH_ERR_BUFFER_READ_ONLY; if (max_size > SSHBUF_SIZE_MAX) return SSH_ERR_NO_BUFFER_SPACE; @@ -219,7 +270,7 @@ sshbuf_len(const struct sshbuf *buf) size_t sshbuf_avail(const struct sshbuf *buf) { - if (sshbuf_check_sanity(buf) != 0 || buf->readonly) + if (sshbuf_check_sanity(buf) != 0 || buf->readonly || buf->refcount > 1) return 0; return buf->max_size - (buf->size - buf->off); } @@ -235,7 +286,7 @@ sshbuf_ptr(const struct sshbuf *buf) u_char * sshbuf_mutable_ptr(const struct sshbuf *buf) { - if (sshbuf_check_sanity(buf) != 0 || buf->readonly) + if (sshbuf_check_sanity(buf) != 0 || buf->readonly || buf->refcount > 1) return NULL; return buf->d + buf->off; } @@ -245,9 +296,9 @@ sshbuf_check_reserve(const struct sshbuf *buf, size_t len) { int r; - if ((r = sshbuf_check_sanity(buf)) < 0) + if ((r = sshbuf_check_sanity(buf)) != 0) return r; - if (buf->readonly) + if (buf->readonly || buf->refcount > 1) return SSH_ERR_BUFFER_READ_ONLY; SSHBUF_TELL("check"); /* Slightly odd test construction is to prevent unsigned overflows */ @@ -264,7 +315,7 @@ sshbuf_reserve(struct sshbuf *buf, size_t len, u_char **dpp) int r; SSHBUF_DBG(("reserve buf = %p len = %zu", buf, len)); - if ((r = sshbuf_check_reserve(buf, len)) < 0) { /* does sanity check */ + if ((r = sshbuf_check_reserve(buf, len)) != 0) { if (dpp != NULL) *dpp = NULL; return r; @@ -312,7 +363,7 @@ sshbuf_consume(struct sshbuf *buf, size_t len) int r; SSHBUF_DBG(("len = %zu", len)); - if ((r = sshbuf_check_sanity(buf)) < 0) + if ((r = sshbuf_check_sanity(buf)) != 0) return r; if (len == 0) return 0; @@ -329,7 +380,7 @@ sshbuf_consume_end(struct sshbuf *buf, size_t len) int r; SSHBUF_DBG(("len = %zu", len)); - if ((r = sshbuf_check_sanity(buf)) < 0) + if ((r = sshbuf_check_sanity(buf)) != 0) return r; if (len == 0) return 0; diff --git a/ssh/sshbuf.h b/ssh/sshbuf.h index 72d66b9..c157fe7 100644 --- a/ssh/sshbuf.h +++ b/ssh/sshbuf.h @@ -29,6 +29,7 @@ #endif #define SSHBUF_SIZE_MAX 0x20000000 /* Hard maximum size */ +#define SSHBUF_REFS_MAX 0x100000 /* Max child buffers */ #define SSHBUF_MAX_BIGNUM (8192 / 8) /* Max bignum *bytes* */ #define SSHBUF_MAX_ECPOINT ((528 * 2 / 8) + 1) /* Max EC point *bytes* */ @@ -52,7 +53,7 @@ struct sshbuf *sshbuf_from(const void *blob, size_t len); * resultant buffer. * Returns pointer to buffer on success, or NULL on allocation failure. */ -struct sshbuf *sshbuf_fromb(const struct sshbuf *buf); +struct sshbuf *sshbuf_fromb(struct sshbuf *buf); /* * Create a new, read-only sshbuf buffer from the contents of a string in @@ -255,6 +256,21 @@ int sshbuf_b64tod(struct sshbuf *buf, const char *b64); */ size_t sshbuf_alloc(const struct sshbuf *buf); +/* + * Increment the reference count of buf. + */ +int sshbuf_set_parent(struct sshbuf *child, struct sshbuf *parent); + +/* + * Return the parent buffer of buf, or NULL if it has no parent. + */ +const struct sshbuf *sshbuf_parent(const struct sshbuf *buf); + +/* + * Return the reference count of buf + */ +u_int sshbuf_refcount(const struct sshbuf *buf); + # define SSHBUF_SIZE_INIT 256 /* Initial allocation */ # define SSHBUF_SIZE_INC 256 /* Preferred increment length */ # define SSHBUF_PACK_MIN 8192 /* Minimim packable offset */ diff --git a/unittests/sshbuf/test_sshbuf_fixed.c b/unittests/sshbuf/test_sshbuf_fixed.c index 7c8b7af..10403b7 100644 --- a/unittests/sshbuf/test_sshbuf_fixed.c +++ b/unittests/sshbuf/test_sshbuf_fixed.c @@ -14,6 +14,7 @@ #include "test_helper.h" +#define SSHBUF_INTERNAL 1 /* access internals for testing */ #include "sshbuf.h" #include "err.h" @@ -60,6 +61,35 @@ sshbuf_fixed(void) sshbuf_free(p1); TEST_DONE(); + TEST_START("sshbuf_fromb "); + p1 = sshbuf_new(); + ASSERT_PTR_NE(p1, NULL); + ASSERT_U_INT_EQ(sshbuf_refcount(p1), 1); + ASSERT_PTR_EQ(sshbuf_parent(p1), NULL); + ASSERT_INT_EQ(sshbuf_put(p1, test_buf, sizeof(test_buf) - 1), 0); + p2 = sshbuf_fromb(p1); + ASSERT_PTR_NE(p2, NULL); + ASSERT_U_INT_EQ(sshbuf_refcount(p1), 2); + ASSERT_PTR_EQ(sshbuf_parent(p1), NULL); + ASSERT_PTR_EQ(sshbuf_parent(p2), p1); + ASSERT_PTR_EQ(sshbuf_ptr(p2), sshbuf_ptr(p1)); + ASSERT_PTR_EQ(sshbuf_mutable_ptr(p2), NULL); + ASSERT_SIZE_T_EQ(sshbuf_len(p1), sshbuf_len(p2)); + ASSERT_INT_EQ(sshbuf_get_u8(p2, &c), 0); + ASSERT_PTR_EQ(sshbuf_ptr(p2), sshbuf_ptr(p1) + 1); + ASSERT_U8_EQ(c, 1); + ASSERT_INT_EQ(sshbuf_get_u32(p2, &i), 0); + ASSERT_PTR_EQ(sshbuf_ptr(p2), sshbuf_ptr(p1) + 5); + ASSERT_U32_EQ(i, 0x12345678); + ASSERT_INT_EQ(sshbuf_get_cstring(p2, &s, &l), 0); + ASSERT_SIZE_T_EQ(sshbuf_len(p2), 0); + ASSERT_STRING_EQ(s, "hello"); + ASSERT_SIZE_T_EQ(l, 5); + sshbuf_free(p1); + ASSERT_U_INT_EQ(sshbuf_refcount(p1), 1); + sshbuf_free(p2); + TEST_DONE(); + TEST_START("sshbuf_froms"); p1 = sshbuf_new(); ASSERT_PTR_NE(p1, NULL);