Make sure strings are terminated before callng regexec.
If they are we can simply copy the pointers, if they
aren't we need to copy the string into a new block
of memory.
Fix a boundary error in data_string_new()
the software to fail.
[ISC-Bugs #40371]
+- Properly terminate strings before passing them to regex and fix
+ a boudnary error when creating certain new data strings.
+ Thanks to Andrey Jr. Melnikov for the bug report.
+ [ISC-Bugs #41217]
+
Changes since 4.3.3b1
- None
Memory allocation... */
/*
- * Copyright (c) 2009,2013-2014 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2009,2013-2014,2016 by Internet Systems Consortium, Inc. ("ISC")
* Copyright (c) 2004-2007 by Internet Systems Consortium, Inc. ("ISC")
* Copyright (c) 1996-2003 by Internet Software Consortium
*
if (group -> object)
group_object_dereference (&group -> object, file, line);
- if (group -> subnet)
+ if (group -> subnet)
subnet_dereference (&group -> subnet, file, line);
if (group -> shared_network)
shared_network_dereference (&group -> shared_network,
#endif
struct binding_value *free_binding_values;
-
+
int binding_value_allocate (cptr, file, line)
struct binding_value **cptr;
const char *file;
{
struct buffer *bp;
- /* XXXSK: should check for bad ptr values, otherwise we
+ /* XXXSK: should check for bad ptr values, otherwise we
leak memory if they are wrong */
bp = dmalloc (len + sizeof *bp, file, line);
if (!bp)
if (new_string == NULL) {
log_error("data_string_new: new_string cannot be NULL %s(%d)",
file, line);
- return (0);
+ return (0);
}
if (src == NULL) {
log_error("data_string_new: src cannot be NULL %s(%d)",
file, line);
- return (0);
+ return (0);
}
memset(new_string, 0, sizeof (struct data_string));
/* If we already have a NULL back off length by one. This lets
* us always just add a NULL at the end. */
- copy_len = (len > 0 && src[len - 1 ] == 0) ? len - 1 : len;
+ copy_len = (len > 0 && src[len - 1] == 0) ? len - 1 : len;
/* Allocate the buffer, accounting for terminating null */
if (!buffer_allocate(&(new_string->buffer), copy_len + 1, MDL)) {
log_error("data_string_new: No memory %s(%d)", file, line);
- return (0);
+ return (0);
}
/* Only copy if there's something to copy */
}
/* Always tack on the null */
- new_string->buffer->data[copy_len + 1] = 0;
+ new_string->buffer->data[copy_len] = 0;
/* Update data_string accessor values. Note len does NOT include
* the NULL. */
memset (data, 0, sizeof *data);
}
-/* If the data_string is larger than the specified length, reduce
+/* If the data_string is larger than the specified length, reduce
the data_string to the specified size. */
void data_string_truncate (dp, len)
dp -> len = len;
}
}
+
+/* \brief Converts a data_string to a null-terminated data string
+ *
+ * If the given string isn't null-terminated, replace it with a
+ * null-terminated version and free the current string decrementing
+ * the referecne count. If the string is null-terminated it is left
+ * as is.
+ *
+ * Currently this routine doesn't check if the string is 0 length
+ * that must be checked by the caller.
+ *
+ * \param [in/out] str the data_string to convert
+ * \param file the file this routine was called from
+ * \param line the line this routine was called from
+ *
+ * \return 1 if the string was converted successfully (or already terminated),
+ * 0 if the conversion failed. Failure is only possible if memory for the new
+ * string could not be allocated. If the conversion fails, the original
+ * string's content is lost.
+ */
+int data_string_terminate(str, file, line)
+ struct data_string* str;
+ const char *file;
+ int line;
+{
+ int ret_val = 1;
+
+ if (str->terminated == 0) {
+ struct data_string temp;
+ memset(&temp, 0, sizeof(temp));
+
+ data_string_copy(&temp, str, file, line);
+ data_string_forget(str, file, line);
+ if (data_string_new(str, (const char*)temp.data, temp.len,
+ file, line) == 0) {
+ /* couldn't create a copy, probably a memory issue,
+ * an error message has already been logged. */
+ ret_val = 0;
+ }
+
+ /* get rid of temp string */
+ data_string_forget(&temp, file, line);
+ }
+
+ return (ret_val);
+}
/*
- * Copyright (c) 2007,2009-2014 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2007,2009-2014,2016 by Internet Systems Consortium, Inc. ("ISC")
*
* We test the functions provided in alloc.c here. These are very
* basic functions, and it is very important that they work correctly.
return (NULL);
}
+ATF_TC(data_string_terminate);
+
+ATF_TC_HEAD(data_string_terminate, tc) {
+ atf_tc_set_md_var(tc, "descr", "data_string_terminate test, "
+ "exercises data_string_terminate function");
+}
+
+ATF_TC_BODY(data_string_terminate, tc) {
+ struct data_string new_string, copy_string;
+ const char *src = "Boring test string";
+
+ /* Case 1: Call with an already terminated string. The
+ * original structure shouldn't be touched.
+ */
+ memset(&new_string, 0, sizeof(new_string));
+ memset(©_string, 0, sizeof(copy_string));
+ if (data_string_new(&new_string, src, strlen(src), MDL) == 0) {
+ atf_tc_fail("Case 1: unable to create new string");
+ }
+ memcpy(©_string, &new_string, sizeof(new_string));
+ if (data_string_terminate(&new_string, MDL) == 0) {
+ atf_tc_fail("Case 1: unable to terminate string");
+ }
+ if (memcmp(©_string, &new_string, sizeof(new_string)) != 0) {
+ atf_tc_fail("Case 1: structure modified");
+ }
+
+ /* Case 2: Call with an unterminated string. The
+ * original structure should be modified with a pointer
+ * to new memory for the string.
+ */
+ /* clear the termination flag, and shrink the string */
+ new_string.terminated = 0;
+ new_string.len -= 2;
+ memcpy(©_string, &new_string, sizeof(new_string));
+
+ if (data_string_terminate(&new_string, MDL) == 0) {
+ atf_tc_fail("Case 2: unable to terminate string");
+ }
+
+ /* We expect the same string but in a differnet block of memory */
+ if ((new_string.terminated == 0) ||
+ (&new_string.buffer == ©_string.buffer) ||
+ (new_string.len != copy_string.len) ||
+ memcmp(new_string.data, src, new_string.len) ||
+ new_string.data[new_string.len] != 0) {
+ atf_tc_fail("Case 2: structure not modified correctly");
+ }
+
+ /* get rid of the string, no need to get rid of copy as the
+ * string memory was freed during the terminate call */
+ data_string_forget(&new_string, MDL);
+}
ATF_TP_ADD_TCS(tp)
{
ATF_TP_ADD_TC(tp, data_string_copy);
ATF_TP_ADD_TC(tp, data_string_copy_nobuf);
ATF_TP_ADD_TC(tp, data_string_new);
+ ATF_TP_ADD_TC(tp, data_string_terminate);
return (atf_no_error());
}
Routines for manipulating parse trees... */
/*
- * Copyright (c) 2011-2014 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2011-2014,2016 by Internet Systems Consortium, Inc. ("ISC")
* Copyright (c) 2004-2007,2009 by Internet Systems Consortium, Inc. ("ISC")
* Copyright (c) 1995-2003 by Internet Software Consortium
*
in_options, cfg_options,
scope,
expr->data.equal[0], MDL);
+
+ /* This is annoying, regexec requires the string being processed
+ * to be NULL terminated, but left may not be, so pass it into
+ * the termination function to ensure it's null terminated.
+ */
+ if (bleft && (data_string_terminate(&left, MDL) == 0)) {
+ /* failed to make a null terminated version, couldn't
+ * create a copy, probably a memory issue, an error
+ * message has already been logged */
+ bleft = 0;
+ }
+
memset(&right, 0, sizeof right);
bright = evaluate_data_expression(&right, packet, lease,
client_state,
memset(&re, 0, sizeof(re));
if (bleft && bright &&
(left.data != NULL) && (right.data != NULL) &&
- (regcomp(&re, (char *)right.data, regflags) == 0) &&
+ (regcomp(&re, (char *)right.data, regflags) == 0) &&
(regexec(&re, (char *)left.data, (size_t)0, NULL, 0) == 0))
*result = 1;
* If we have bleft and bright then we have a good
* syntax, otherwise not.
*
- * XXX: we don't warn on invalid regular expression
+ * XXX: we don't warn on invalid regular expression
* syntax, should we?
*/
return bleft && bright;
const char *, int);
void data_string_forget (struct data_string *, const char *, int);
void data_string_truncate (struct data_string *, int);
+int data_string_terminate (struct data_string *, const char *, int);
int executable_statement_allocate (struct executable_statement **,
const char *, int);
int executable_statement_reference (struct executable_statement **,