Skip to content

Commit

Permalink
Add unpackstr_xmalloc_escaped and unpackstr_xmalloc_chooser.
Browse files Browse the repository at this point in the history
Use unpackstr_xmalloc_chooser as part of safe_unpackstr_xmalloc macro.
This ensures that the slurmdbd is always working on escaped strings.

Fixes CVE-2018-7033.

Bug 4792.
  • Loading branch information
dannyauble authored and wickberg committed Mar 15, 2018
1 parent 0d9b032 commit db46889
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 2 deletions.
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ documents those changes that are of interest to users and administrators.
-- Fix whole node allocation cpu counts when --hint=nomultihtread.
-- NRT - Fix issue when running on a HFI (p775) system with multiple protocols.
-- Fix uninitialized variables when unpacking slurmdb_archive_cond_t.
-- Fix security issue in accounting_storage/mysql plugin by always escaping
strings within the slurmdbd. CVE-2018-7033.

* Changes in Slurm 17.02.9
==========================
Expand Down
76 changes: 76 additions & 0 deletions src/common/pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
#include "src/common/macros.h"
#include "src/common/pack.h"
#include "src/common/xmalloc.h"
#include "src/slurmdbd/read_config.h"

/*
* Define slurm-specific aliases for use by plugins, see slurm_xlator.h
Expand Down Expand Up @@ -85,6 +86,8 @@ strong_alias(unpackmem, slurm_unpackmem);
strong_alias(unpackmem_ptr, slurm_unpackmem_ptr);
strong_alias(unpackmem_xmalloc, slurm_unpackmem_xmalloc);
strong_alias(unpackmem_malloc, slurm_unpackmem_malloc);
strong_alias(unpackstr_xmalloc_escaped, slurm_unpackstr_xmalloc_escaped);
strong_alias(unpackstr_xmalloc_chooser, slurm_unpackstr_xmalloc_chooser);
strong_alias(packstr_array, slurm_packstr_array);
strong_alias(unpackstr_array, slurm_unpackstr_array);
strong_alias(packmem_array, slurm_packmem_array);
Expand Down Expand Up @@ -801,6 +804,79 @@ int unpackmem_malloc(char **valp, uint32_t * size_valp, Buf buffer)
return SLURM_SUCCESS;
}

/*
* Given a buffer containing a network byte order 16-bit integer,
* and an arbitrary char string, copy the data string into the location
* specified by valp and escape ' and \ to be database safe.
* Also return the sizes of 'valp' in bytes.
* Adjust buffer counters.
* NOTE: valp is set to point into a newly created buffer,
* the caller is responsible for calling xfree() on *valp
* if non-NULL (set to NULL on zero size buffer value)
* NOTE: size_valp may not match how much data was processed from buffer, but
* will match the length of the returned 'valp'.
* WARNING: These escapes are sufficient to protect MariaDB/MySQL, but
* may not be sufficient if databases are added in the future.
*/
int unpackstr_xmalloc_escaped(char **valp, uint32_t *size_valp, Buf buffer)
{
uint32_t ns;

if (remaining_buf(buffer) < sizeof(ns))
return SLURM_ERROR;

memcpy(&ns, &buffer->head[buffer->processed], sizeof(ns));
*size_valp = ntohl(ns);
buffer->processed += sizeof(ns);

if (*size_valp > MAX_PACK_MEM_LEN) {
error("%s: Buffer to be unpacked is too large (%u > %u)",
__func__, *size_valp, MAX_PACK_MEM_LEN);
return SLURM_ERROR;
} else if (*size_valp > 0) {
uint32_t cnt = *size_valp;

if (remaining_buf(buffer) < cnt)
return SLURM_ERROR;

/* make a buffer 2 times the size just to be safe */
*valp = xmalloc_nz((cnt * 2) + 1);
if (*valp) {
char *copy = NULL, *str, tmp;
uint32_t i;
copy = *valp;
str = &buffer->head[buffer->processed];

for (i = 0; i < cnt && *str; i++) {
tmp = *str++;
if ((tmp == '\\') || (tmp == '\'')) {
*copy++ = '\\';
(*size_valp)++;
}

*copy++ = tmp;
}

/* Since we used xmalloc_nz, terminate the string. */
*copy++ = '\0';
}

/* add the original value since that is what we processed */
buffer->processed += cnt;
} else
*valp = NULL;
return SLURM_SUCCESS;
}

int unpackstr_xmalloc_chooser(char **valp, uint32_t *size_valp, Buf buf)
{
if (slurmdbd_conf)
return unpackstr_xmalloc_escaped(valp, size_valp, buf);
else
return unpackmem_xmalloc(valp, size_valp, buf);
}


/*
* Given a pointer to array of char * (char ** or char *[] ) and a size
* (size_val), convert size_val to network byte order and store in the
Expand Down
11 changes: 9 additions & 2 deletions src/common/pack.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ int unpackmem_ptr(char **valp, uint32_t *size_valp, Buf buffer);
int unpackmem_xmalloc(char **valp, uint32_t *size_valp, Buf buffer);
int unpackmem_malloc(char **valp, uint32_t *size_valp, Buf buffer);

int unpackstr_xmalloc_escaped(char **valp, uint32_t *size_valp, Buf buffer);
int unpackstr_xmalloc_chooser(char **valp, uint32_t *size_valp, Buf buffer);

void packstr_array(char **valp, uint32_t size_val, Buf buffer);
int unpackstr_array(char ***valp, uint32_t* size_val, Buf buffer);

Expand Down Expand Up @@ -333,8 +336,12 @@ int unpackmem_array(char *valp, uint32_t size_valp, Buf buffer);
#define safe_unpackstr_malloc \
safe_unpackmem_malloc

#define safe_unpackstr_xmalloc \
safe_unpackmem_xmalloc
#define safe_unpackstr_xmalloc(valp, size_valp, buf) do { \
assert(sizeof(*size_valp) == sizeof(uint32_t)); \
assert(buf->magic == BUF_MAGIC); \
if (unpackstr_xmalloc_chooser(valp, size_valp, buf)) \
goto unpack_error; \
} while (0)

#define safe_unpackstr_array(valp,size_valp,buf) do { \
assert(sizeof(*size_valp) == sizeof(uint32_t)); \
Expand Down
2 changes: 2 additions & 0 deletions src/common/slurm_xlator.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,8 @@
#define unpackmem_ptr slurm_unpackmem_ptr
#define unpackmem_xmalloc slurm_unpackmem_xmalloc
#define unpackmem_malloc slurm_unpackmem_malloc
#define unpackstr_xmalloc_escaped slurm_unpackstr_xmalloc_escaped
#define unpackstr_xmalloc_chooser slurm_unpackstr_xmalloc_chooser
#define packstr_array slurm_packstr_array
#define unpackstr_array slurm_unpackstr_array
#define packmem_array slurm_packmem_array
Expand Down

0 comments on commit db46889

Please sign in to comment.