diff --git a/amd_openvx/openvx/ago/ago_interface.cpp b/amd_openvx/openvx/ago/ago_interface.cpp index 2e5e88a21..7533e3a73 100644 --- a/amd_openvx/openvx/ago/ago_interface.cpp +++ b/amd_openvx/openvx/ago/ago_interface.cpp @@ -744,7 +744,7 @@ static void agoReadGraphFromStringInternal(AgoGraph * agraph, AgoReference * * r group = atoi(&szGroup[1]); } node->attr_affinity.group = group; - (void)sscanf(&arg[2 + p][14], "%s", device); + (void)sscanf(&arg[2 + p][14], "%63s", device); if (!strncmp(device, "CPU", 3)) { node->attr_affinity.device_type = AGO_KERNEL_FLAG_DEVICE_CPU; node->attr_affinity.device_info = atoi(&device[3]); @@ -782,11 +782,11 @@ static void agoReadGraphFromStringInternal(AgoGraph * agraph, AgoReference * * r } } else if (strcmp(arg[2 + p], "null") != 0) { - char name[128]; strcpy(name, arg[2 + p]); + char name[128]; snprintf(name, sizeof(name), "%s", arg[2 + p]); // check if there is an name alias for (std::vector< std::pair< std::string, std::string > >::iterator it = aliases.begin(); it != aliases.end(); ++it) { if (!strcmp(it->first.c_str(), name)) { - strcpy(name, it->second.c_str()); + snprintf(name, sizeof(name), "%s", it->second.c_str()); if (name[0] == '$') { int index = atoi(&name[1]) - 1; if (index >= 0 && index < num_ref) { @@ -824,6 +824,11 @@ static void agoReadGraphFromStringInternal(AgoGraph * agraph, AgoReference * * r } if (data) { if (node) { + if (p >= AGO_MAX_PARAMS) { + agoAddLogEntry(&agraph->ref, VX_FAILURE, "ERROR: agoReadGraph: line %d: number of arguments exceeded MAX(%d)\n>>>> %s\n", lineno, AGO_MAX_PARAMS, lineCopy); + agraph->status = -1; + break; + } node->paramList[p] = data; // check if specified data type is correct // NOTE: kernel can specify to ignore this checking by setting argType[] to ZERO @@ -885,7 +890,7 @@ static void agoReadGraphFromStringInternal(AgoGraph * agraph, AgoReference * * r if (dumpToConsole) agoAddLogEntry(NULL, VX_SUCCESS, "%s", line); agoUpdateLine(line, vars, localPrefix); char word[256]; - if (sscanf(line, "%s", word) == 1 && !strcmp(word, "endmacro")) + if (sscanf(line, "%255s", word) == 1 && !strcmp(word, "endmacro")) break; str_end = str; } @@ -1157,6 +1162,12 @@ int agoLoadModule(AgoContext * context, const char * module) if (agoIsValidContext(context)) { CAgoLock lock(context->cs); status = VX_ERROR_INVALID_PARAMETERS; + // Module names resolve to lib.so via the loader search path; reject + // path separators and ".." so only a bare module name can be loaded. + if (!module || strpbrk(module, "/\\") || strstr(module, "..")) { + agoAddLogEntry(&context->ref, VX_FAILURE, "ERROR: agoLoadModule: invalid module name '%s' (path separators and '..' are not allowed)\n", module ? module : "(null)"); + return VX_ERROR_INVALID_PARAMETERS; + } char filePath[1024]; snprintf(filePath, sizeof(filePath), SHARED_LIBRARY_PREFIX "%s" SHARED_LIBRARY_EXTENSION, module); for (vx_uint32 index = 0; index < context->num_active_modules; index++) { if (strcmp(filePath, context->modules[index].module_path) == 0) { diff --git a/amd_openvx/openvx/ago/ago_util.cpp b/amd_openvx/openvx/ago/ago_util.cpp index b48374412..7893bd220 100644 --- a/amd_openvx/openvx/ago/ago_util.cpp +++ b/amd_openvx/openvx/ago/ago_util.cpp @@ -707,10 +707,10 @@ AgoKernel * agoFindKernelByName(AgoContext * acontext, const vx_char * name) AgoData * agoFindDataByName(AgoContext * acontext, AgoGraph * agraph, vx_char * name) { // check for [index] syntax - char actualName[256]; strcpy(actualName, name); + char actualName[256]; snprintf(actualName, sizeof(actualName), "%s", name); int index[4] = { -1, -1, -1, -1 }; // index >=0 indicates special object const char * s = strstr(name, "["); - if (s && name[strlen(name) - 1] == ']') { + if (s && name[strlen(name) - 1] == ']' && (size_t)(s - name) < sizeof(actualName)) { actualName[s - name] = 0; for (int i = 0; i < 4 && *s == '['; i++) { s++; if (*s == '-') s++; @@ -1245,6 +1245,7 @@ int agoGetDataFromDescription(AgoContext * acontext, AgoGraph * agraph, AgoData //data->u.img.isROI = vx_true_e; const char *s = strstr(desc, ","); if (!s) return -1; char master_name[128]; + if ((size_t)(s - desc) >= sizeof(master_name)) return -1; memcpy(master_name, desc, s - desc); master_name[s - desc] = 0; s++; if (sscanf(s, "%u,%u,%u,%u", &data->u.img.rect_roi.start_x, &data->u.img.rect_roi.start_y, &data->u.img.rect_roi.end_x, &data->u.img.rect_roi.end_y) != 4) return -1; @@ -1335,7 +1336,7 @@ int agoGetDataFromDescription(AgoContext * acontext, AgoGraph * agraph, AgoData data->ref.type = VX_TYPE_PYRAMID; memcpy(&data->u.pyr.format, desc, sizeof(data->u.pyr.format)); char scale[64] = ""; - if (sscanf(desc + 5, "%d,%d," VX_FMT_SIZE ",%s", &data->u.pyr.width, &data->u.pyr.height, &data->u.pyr.levels, scale) != 4) return -1; + if (sscanf(desc + 5, "%d,%d," VX_FMT_SIZE ",%63s", &data->u.pyr.width, &data->u.pyr.height, &data->u.pyr.levels, scale) != 4) return -1; if (!strncmp(scale, "HALF", 4)) data->u.pyr.scale = VX_SCALE_PYRAMID_HALF; else if (!strncmp(scale, "ORB", 3)) data->u.pyr.scale = VX_SCALE_PYRAMID_ORB; else data->u.pyr.scale = (vx_float32)atof(scale); @@ -1394,6 +1395,7 @@ int agoGetDataFromDescription(AgoContext * acontext, AgoGraph * agraph, AgoData data->ref.type = VX_TYPE_ARRAY; const char *s = strstr(desc, ","); if (!s) return -1; char data_type[64]; + if ((size_t)(s - desc) >= sizeof(data_type)) return -1; memcpy(data_type, desc, s - desc); data_type[s - desc] = 0; (void)sscanf(++s, "" VX_FMT_SIZE "", &data->u.arr.capacity); data->u.arr.itemtype = agoName2Enum(data_type); @@ -1472,6 +1474,7 @@ int agoGetDataFromDescription(AgoContext * acontext, AgoGraph * agraph, AgoData data->ref.type = VX_TYPE_OBJECT_ARRAY; const char *s = strstr(desc, ","); if (!s) return -1; char data_type[64]; + if ((size_t)(s - desc) >= sizeof(data_type)) return -1; memcpy(data_type, desc, s - desc); data_type[s - desc] = 0; (void)sscanf(++s, "" VX_FMT_SIZE "", &data->u.objarr.numitems); data->u.objarr.itemtype = agoName2Enum(data_type); @@ -1522,6 +1525,7 @@ int agoGetDataFromDescription(AgoContext * acontext, AgoGraph * agraph, AgoData data->ref.type = VX_TYPE_LUT; const char *s = strstr(desc, ","); if (!s) return -1; char data_type[64]; + if ((size_t)(s - desc) >= sizeof(data_type)) return -1; memcpy(data_type, desc, s - desc); data_type[s - desc] = 0; data->u.lut.type = agoName2Enum(data_type); if (data->u.lut.type != VX_TYPE_UINT8 && data->u.lut.type != VX_TYPE_INT16) @@ -1670,6 +1674,7 @@ int agoGetDataFromDescription(AgoContext * acontext, AgoGraph * agraph, AgoData data->ref.type = VX_TYPE_MATRIX; const char *s = strstr(desc, ","); if (!s) return -1; char data_type[64]; + if ((size_t)(s - desc) >= sizeof(data_type)) return -1; memcpy(data_type, desc, s - desc); data_type[s - desc] = 0; data->u.mat.type = agoName2Enum(data_type); if (data->u.mat.type != VX_TYPE_INT32 && data->u.mat.type != VX_TYPE_FLOAT32 && data->u.mat.type != VX_TYPE_UINT8) { @@ -1715,6 +1720,7 @@ int agoGetDataFromDescription(AgoContext * acontext, AgoGraph * agraph, AgoData data->ref.type = VX_TYPE_SCALAR; const char *s = strstr(desc, ","); if (!s) return -1; char data_type[64]; + if ((size_t)(s - desc) >= sizeof(data_type)) return -1; memcpy(data_type, desc, s - desc); data_type[s - desc] = 0; s++; data->u.scalar.type = agoName2Enum(data_type); @@ -1879,7 +1885,8 @@ int agoGetDataFromDescription(AgoContext * acontext, AgoGraph * agraph, AgoData data->u.tensor.start[i] = 0; data->u.tensor.end[i] = data->u.tensor.dims[i]; data->u.tensor.stride[i] = data->size; - data->size *= data->u.tensor.dims[i]; + if (__builtin_mul_overflow(data->size, (vx_size)data->u.tensor.dims[i], &data->size)) + return -1; // make sure that the size and stride[1] are multiple of 4:: commending out since it breaks fp16 /* if (i == 0 && (data->size & 3)) { @@ -1952,7 +1959,8 @@ int agoGetDataFromDescription(AgoContext * acontext, AgoGraph * agraph, AgoData data->u.tensor.dims[i] = end[i] - start[i]; data->u.tensor.stride[i] = dataMaster->u.tensor.stride[i]; data->u.tensor.offset += start[i] * dataMaster->u.tensor.stride[i]; - data->size *= data->u.tensor.dims[i]; + if (__builtin_mul_overflow(data->size, (vx_size)data->u.tensor.dims[i], &data->size)) + return -1; } for (vx_size i = data->u.tensor.num_dims; i < AGO_MAX_TENSOR_DIMENSIONS; i++) { data->u.tensor.start[i] = 0; @@ -2426,10 +2434,12 @@ int agoDataSanityCheckAndUpdate(AgoData * data) // - make sure that the stride is multiple of 16 bytes if (data->import_type == VX_IMPORT_TYPE_NONE) { data->u.img.stride_in_bytes = ALIGN16(ImageWidthInBytesCeil(data->u.img.width, data)); - data->size = ALIGN16(data->u.img.height) * data->u.img.stride_in_bytes; + if (__builtin_mul_overflow((vx_size)ALIGN16(data->u.img.height), (vx_size)data->u.img.stride_in_bytes, &data->size)) + return -1; } else { - data->size = (vx_size)((unsigned long)(data->u.img.height * data->u.img.stride_in_bytes)); + if (__builtin_mul_overflow((vx_size)data->u.img.height, (vx_size)data->u.img.stride_in_bytes, &data->size)) + return -1; } if (!data->size) return -1; @@ -2549,7 +2559,11 @@ int agoDataSanityCheckAndUpdate(AgoData * data) else data->u.remap.remap_fractional_bits = AGO_REMAP_FRACTIONAL_BITS; // calculate other attributes and buffer size - data->size = (vx_size)((unsigned long)(data->u.remap.dst_width * data->u.remap.dst_height) * sizeof(ago_coord2d_ushort_t)); + vx_size remap_bytes; + if (__builtin_mul_overflow((vx_size)data->u.remap.dst_width, (vx_size)data->u.remap.dst_height, &remap_bytes) || + __builtin_mul_overflow(remap_bytes, (vx_size)sizeof(ago_coord2d_ushort_t), &remap_bytes)) + return -1; + data->size = remap_bytes; if (!data->size) return -1; } diff --git a/utilities/runvx/vxImage.cpp b/utilities/runvx/vxImage.cpp index 251b9d05a..70f534f86 100644 --- a/utilities/runvx/vxImage.cpp +++ b/utilities/runvx/vxImage.cpp @@ -23,6 +23,23 @@ THE SOFTWARE. #define _CRT_SECURE_NO_WARNINGS #include "vxImage.h" +// The filename from the GDF is used as an snprintf format string. Allow only integer +// conversions (%d, %03d, ...) and %%; reject everything else so %n and %s can't get in. +static bool IsSafeFilenameTemplate(const char * fmt) +{ + for (const char * p = fmt; *p; p++) { + if (*p != '%') continue; + p++; + if (*p == '%') continue; // literal %% escape + while (*p == '-' || *p == '+' || *p == ' ' || *p == '#' || *p == '0') p++; // flags + while (*p >= '0' && *p <= '9') p++; // field width + if (*p == '.') { p++; while (*p >= '0' && *p <= '9') p++; } // precision + while (*p == 'l' || *p == 'h') p++; // length modifiers + if (!strchr("diouxX", *p)) return false; // only integer conversions allowed + } + return true; +} + /////////////////////////////////////////////////////////////////////// // class CVxParamImage // @@ -797,6 +814,7 @@ int CVxParamImage::ReadFrame(int frameNumber) if (!m_fpRead) { if (m_fileNameRead.length() > 0) { char fileName[MAX_FILE_NAME_LENGTH]; + if (!IsSafeFilenameTemplate(m_fileNameRead.c_str())) ReportError("ERROR: unsafe filename template: %s\n", m_fileNameRead.c_str()); snprintf(fileName, sizeof(fileName), m_fileNameRead.c_str(), frameNumber, m_width, m_height); m_fpRead = fopen(fileName, "rb"); if (!m_fpRead) ReportError("ERROR: unable to open: %s\n", fileName); if (!m_fileNameForReadHasIndex && m_captureFrameStart > 0) { @@ -1082,6 +1100,7 @@ int CVxParamImage::ViewFrame(int frameNumber) colorIndex++; // get list of keypoints from the user specified file char fileName[512]; + if (!IsSafeFilenameTemplate(it->c_str())) ReportError("ERROR: unsafe filename template: %s\n", it->c_str()); snprintf(fileName, sizeof(fileName), it->c_str(), frameNumber); FILE * fp = fopen(fileName, "r"); if (!fp) ReportError("ERROR: unable to open '%s'\n", fileName); @@ -1154,6 +1173,7 @@ int CVxParamImage::WriteFrame(int frameNumber) if (!m_fpWrite) { if (m_fileNameWrite.length() > 0 && !m_usingWriter) { char fileName[MAX_FILE_NAME_LENGTH]; + if (!IsSafeFilenameTemplate(m_fileNameWrite.c_str())) ReportError("ERROR: unsafe filename template: %s\n", m_fileNameWrite.c_str()); snprintf(fileName, sizeof(fileName), m_fileNameWrite.c_str(), frameNumber, m_width, m_height); #if ENABLE_OPENCV // check if openCV imwrite need to be used