More dns_qpkey_t safety checks
My original idea had been that the core qp-trie code would be mostly independent of the storage for keys, so I did not make it check at run time that key lengths are sensible. However, the qp-trie search routines need to get keys out of leaf objects, for which they provide storage on the stack, which is particularly dangerous for unchecked buffer overflows. So this change checks that key lengths are in bounds at the API boundary between the qp-trie code and the rest of BIND, and there is no more pretence that keys might be longer.
This commit is contained in:
@@ -193,8 +193,9 @@ typedef uint8_t dns_qpkey_t[512];
|
||||
*
|
||||
* The `makekey` method fills in a `dns_qpkey_t` corresponding to a
|
||||
* value object stored in the qp-trie. It returns the length of the
|
||||
* key. This method will typically call dns_qpkey_fromname() with a
|
||||
* name stored in the value object.
|
||||
* key, which must be less than `sizeof(dns_qpkey_t)`. This method
|
||||
* will typically call dns_qpkey_fromname() with a name stored in the
|
||||
* value object.
|
||||
*
|
||||
* For logging and tracing, the `triename` method copies a human-
|
||||
* readable identifier into `buf` which has max length `size`.
|
||||
@@ -395,15 +396,18 @@ dns_qpkey_fromname(dns_qpkey_t key, const dns_name_t *name);
|
||||
* Requires:
|
||||
* \li `name` is a pointer to a valid `dns_name_t`
|
||||
*
|
||||
* Ensures:
|
||||
* \li returned length is less than `sizeof(dns_qpkey_t)`
|
||||
*
|
||||
* Returns:
|
||||
* \li the length of the key
|
||||
*/
|
||||
|
||||
isc_result_t
|
||||
dns_qp_getkey(dns_qpreadable_t qpr, const dns_qpkey_t searchk, size_t searchl,
|
||||
void **pval_r, uint32_t *ival_r);
|
||||
dns_qp_getkey(dns_qpreadable_t qpr, const dns_qpkey_t search_key,
|
||||
size_t search_keylen, void **pval_r, uint32_t *ival_r);
|
||||
/*%<
|
||||
* Find a leaf in a qp-trie that matches the given key
|
||||
* Find a leaf in a qp-trie that matches the given search key
|
||||
*
|
||||
* The leaf values are assigned to `*pval_r` and `*ival_r`
|
||||
*
|
||||
@@ -411,6 +415,7 @@ dns_qp_getkey(dns_qpreadable_t qpr, const dns_qpkey_t searchk, size_t searchl,
|
||||
* \li `qpr` is a pointer to a readable qp-trie
|
||||
* \li `pval_r != NULL`
|
||||
* \li `ival_r != NULL`
|
||||
* \li `search_keylen < sizeof(dns_qpkey_t)`
|
||||
*
|
||||
* Returns:
|
||||
* \li ISC_R_NOTFOUND if the trie has no leaf with a matching key
|
||||
@@ -452,12 +457,13 @@ dns_qp_insert(dns_qp_t *qp, void *pval, uint32_t ival);
|
||||
*/
|
||||
|
||||
isc_result_t
|
||||
dns_qp_deletekey(dns_qp_t *qp, const dns_qpkey_t key, size_t len);
|
||||
dns_qp_deletekey(dns_qp_t *qp, const dns_qpkey_t key, size_t keylen);
|
||||
/*%<
|
||||
* Delete a leaf from a qp-trie that matches the given key
|
||||
*
|
||||
* Requires:
|
||||
* \li `qp` is a pointer to a valid qp-trie
|
||||
* \li `keylen < sizeof(dns_qpkey_t)`
|
||||
*
|
||||
* Returns:
|
||||
* \li ISC_R_NOTFOUND if the trie has no leaf with a matching key
|
||||
|
||||
@@ -242,6 +242,7 @@ dns_qpkey_fromname(dns_qpkey_t key, const dns_name_t *name) {
|
||||
}
|
||||
/* mark end with a double NOBYTE */
|
||||
key[len] = SHIFT_NOBYTE;
|
||||
ENSURE(len < sizeof(dns_qpkey_t));
|
||||
return (len);
|
||||
}
|
||||
|
||||
@@ -1606,6 +1607,7 @@ dns_qp_deletekey(dns_qp_t *qp, const dns_qpkey_t search_key,
|
||||
qp_node_t *n;
|
||||
|
||||
REQUIRE(QP_VALID(qp));
|
||||
REQUIRE(search_keylen < sizeof(dns_qpkey_t));
|
||||
|
||||
if (get_root(qp) == NULL) {
|
||||
return (ISC_R_NOTFOUND);
|
||||
@@ -1696,6 +1698,7 @@ dns_qp_getkey(dns_qpreadable_t qpr, const dns_qpkey_t search_key,
|
||||
REQUIRE(QP_VALID(qp));
|
||||
REQUIRE(pval_r != NULL);
|
||||
REQUIRE(ival_r != NULL);
|
||||
REQUIRE(search_keylen < sizeof(dns_qpkey_t));
|
||||
|
||||
n = get_root(qp);
|
||||
if (n == NULL) {
|
||||
|
||||
@@ -937,8 +937,10 @@ detach_leaf(dns_qpreadable_t qpr, qp_node_t *n) {
|
||||
static inline size_t
|
||||
leaf_qpkey(dns_qpreadable_t qpr, qp_node_t *n, dns_qpkey_t key) {
|
||||
dns_qpreader_t *qp = dns_qpreader(qpr);
|
||||
return (qp->methods->makekey(key, qp->uctx, leaf_pval(n),
|
||||
leaf_ival(n)));
|
||||
size_t len = qp->methods->makekey(key, qp->uctx, leaf_pval(n),
|
||||
leaf_ival(n));
|
||||
INSIST(len < sizeof(dns_qpkey_t));
|
||||
return (len);
|
||||
}
|
||||
|
||||
static inline char *
|
||||
|
||||
Reference in New Issue
Block a user