Skip to content

Fix: security scan issues related to memory safety & input validation #1723

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sruthi0107 wants to merge 3 commits into
base: develop
Choose a base branch
from
+57 −12
Open

Fix: security scan issues related to memory safety & input validation #1723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
641d081
Fix buffer overflows and integer overflows in GDF parsing
sruthi0107 Jul 27, 2026
ff807a1
Merge branch 'develop' into sec-fixes-mythos-ago
kiritigowda Jul 28, 2026
0762366
Merge branch 'develop' into sec-fixes-mythos-ago
kiritigowda Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions amd_openvx/openvx/ago/ago_interface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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<name>.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) {
Expand Down
30 changes: 22 additions & 8 deletions amd_openvx/openvx/ago/ago_util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,10 +707,10 @@ AgoKernel * agoFindKernelByName(AgoContext * acontext, const vx_char * name)
AgoData * agoFindDataByName(AgoContext * acontext, AgoGraph * agraph, vx_char * name)
{
// check for <object>[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++;
Comment on lines 712 to 716
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
20 changes: 20 additions & 0 deletions utilities/runvx/vxImage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Comment on lines +26 to +41

///////////////////////////////////////////////////////////////////////
// class CVxParamImage
//
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
Loading