Skip to content

Commit a46989c

Browse files
andypughandypugh
authored
Merge pull request #3224 from rmu75/rs/fix-3201-potentiel-buffer-overflow
fix potential buffer overflow
2 parents 581e0ae + 156a026 commit a46989c

File tree

1 file changed

+109
-90
lines changed

1 file changed

+109
-90
lines changed

‎src/emc/rs274ngc/interp_remap.cc‎

Lines changed: 109 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ namespace bp = boost::python;
3232
#include "interp_internal.hh"
3333
#include <rtapi_string.h>
3434

35+
#include <string>
36+
#include <string_view>
3537

3638

3739
bool Interp::is_m_code_remappable(int m_code)
@@ -161,7 +163,7 @@ int Interp::convert_remapped_code(block_pointer block,
161163
if (remap->argspec && (strchr(remap->argspec, '@') != NULL)) {
162164
// append a positional argument list instead of local variables
163165
// if user specified '@'
164-
// named local params are dealt with in execute_call() when
166+
// named local params are dealt with in execute_call() when
165167
// the new call frame is fully established
166168
CHP(add_parameters(settings, cblock, &cmd[strlen(cmd)]));
167169
}
@@ -174,7 +176,7 @@ int Interp::convert_remapped_code(block_pointer block,
174176
// good to go, pass to o-word call handling mechanism
175177
status = read(cmd);
176178
block_pointer eblock = &EXECUTING_BLOCK(*settings);
177-
eblock->call_type = CT_REMAP;
179+
eblock->call_type = CT_REMAP;
178180
CHKS(status != INTERP_OK,
179181
"convert_remapped_code: initial read returned %s",
180182
interp_status(status));
@@ -214,14 +216,13 @@ int Interp::add_parameters(setup_pointer settings,
214216
block_pointer cblock,
215217
char *posarglist)
216218
{
217-
const char *s,*argspec, *code;
219+
const char *code;
218220
block_pointer block;
219-
char missing[30],optional[30],required[30];
220-
char *m = missing;
221-
char *o = optional;
222-
char *r = required;
223-
char msg[LINELEN], tail[LINELEN];
221+
std::string missing, optional, required;
222+
std::string msg;
223+
std::string tail;
224224
bool errored = false;
225+
bool interp_error = false;
225226
remap_pointer rptr = cblock->executing_remap;
226227
context_pointer active_frame = &settings->sub_context[settings->call_level];
227228

@@ -233,66 +234,82 @@ int Interp::add_parameters(setup_pointer settings,
233234
// if any Python handlers are present, create a kwargs dict
234235
bool pydict = rptr->remap_py || rptr->prolog_func || rptr->epilog_func;
235236

236-
std::fill(missing, std::end(missing), 0);
237-
std::fill(optional, std::end(optional), 0);
238-
std::fill(required, std::end(required), 0);
239-
std::fill(msg, std::end(msg), 0);
240-
std::fill(tail, std::end(tail), 0);
241-
242-
s = argspec = rptr->argspec;
243-
CHKS((argspec == NULL),"BUG: add_parameters: argspec = NULL");
244-
245-
while (*s) {
246-
if (isupper(*s) && !strchr(required,*s)) *r++ = tolower(*s);
247-
if (islower(*s) && !strchr(optional,*s)) *o++ = *s;
248-
if (strchr(">^Nn",*s) && !strchr(required,*s)) *r++ = *s;
249-
s++;
237+
//argspec = rptr->argspec;
238+
CHKS((rptr->argspec == NULL),"BUG: add_parameters: argspec = NULL");
239+
240+
std::string_view argspec = rptr->argspec;
241+
for (auto s : argspec) {
242+
if (isupper(s) && required.find_first_of(s) == std::string::npos) {
243+
required += tolower(s);
244+
}
245+
if (islower(s) && optional.find_first_of(s) == std::string::npos) {
246+
optional += s;
247+
}
248+
if ((s == '>' || s == '^' || s == 'N' || s == 'n')
249+
&& required.find_first_of(s) == std::string::npos) {
250+
required += s;
251+
}
250252
}
251253
block = &CONTROLLING_BLOCK((*settings));
252254

253255
logNP("add_parameters code=%s argspec=%s call_level=%d r=%s o=%s pydict=%d\n",
254-
code,argspec,settings->call_level,required,optional,pydict);
255-
256-
#define STORE(name,value) \
257-
if (pydict) { \
258-
try { \
259-
active_frame->pystuff.impl->kwargs[name] = value; \
260-
} \
261-
catch (const bp::error_already_set&) { \
262-
PyErr_Print(); \
263-
PyErr_Clear(); \
264-
ERS("add_parameters: can\'t add '%s' to args",name); \
265-
} \
266-
} \
267-
if (posarglist) { \
268-
char actual[LINELEN]; \
269-
snprintf(actual, sizeof(actual),"[%.4lf]", value); \
270-
strcat(posarglist, actual); \
271-
cblock->param_cnt++; \
272-
} else { \
273-
add_named_param(name,0); \
274-
store_named_param(settings,name,value,0); \
275-
}
276-
277-
278-
#define PARAM(spec,name,flag,value) \
279-
if ((flag)) { /* present */ \
280-
/* required or optional */ \
281-
if (strchr(required,spec) || strchr(optional,spec)) { \
282-
STORE(name,value); \
283-
} \
284-
} else { \
285-
if (strchr(required,spec)) { /* missing */ \
286-
*m++ = spec; \
287-
errored = true; \
288-
} \
289-
}
256+
code,
257+
argspec.data(),
258+
settings->call_level,
259+
required.c_str(),
260+
optional.c_str(),
261+
pydict);
262+
263+
auto STORE
264+
{
265+
[&](const char* name, double value) -> void {
266+
if (pydict) {
267+
try {
268+
active_frame->pystuff.impl->kwargs[name] = value;
269+
}
270+
catch (const bp::error_already_set&) {
271+
PyErr_Print();
272+
PyErr_Clear();
273+
ERM("add parameters: can't add '%s' to args", name);
274+
interp_error = true;
275+
return;
276+
}
277+
}
278+
if (posarglist) {
279+
char actual[LINELEN];
280+
snprintf(actual, sizeof(actual), "[%.4lf]", value);
281+
strcat(posarglist, actual);
282+
cblock->param_cnt++;
283+
}
284+
else {
285+
add_named_param(name, 0);
286+
store_named_param(settings, name, value, 0);
287+
}
288+
}
289+
};
290+
291+
auto PARAM
292+
{
293+
[&](char spec, const char* name, bool flag, double value) -> void {
294+
if (flag) {
295+
if (required.find_first_of(spec) != std::string::npos
296+
|| optional.find_first_of(spec) != std::string::npos) {
297+
STORE(name, value);
298+
}
299+
}
300+
else {
301+
if (required.find_first_of(spec) != std::string::npos) {
302+
missing += spec;
303+
errored = true;
304+
}
305+
}
306+
}
307+
};
290308

291-
s = rptr->argspec;
292309
// step through argspec in order so positional args are built
293310
// in the correct order
294-
while (*s) {
295-
switch (tolower(*s)) {
311+
for (auto s: argspec) {
312+
switch (tolower(s)) {
296313
case 'a' : PARAM('a',"a",block->a_flag,block->a_number); break;
297314
case 'b' : PARAM('b',"b",block->b_flag,block->b_number); break;
298315
case 'c' : PARAM('c',"c",block->c_flag,block->c_number); break;
@@ -318,49 +335,51 @@ int Interp::add_parameters(setup_pointer settings,
318335
case '-' : break; // ignore - backwards compatibility
319336
default: ;
320337
}
321-
s++;
338+
if (interp_error)
339+
return INTERP_ERROR;
322340
}
323341

324-
s = missing;
325-
if (*s) {
326-
rtapi_strxcat(tail," missing: ");
342+
if (!missing.empty()) {
343+
tail = " missing: ";
344+
errored = true;
327345
}
328-
while (*s) {
329-
errored = true;
330-
char c = toupper(*s);
331-
strncat(tail,&c,1);
332-
if (*(s+1)) rtapi_strxcat(tail,",");
333-
s++;
346+
bool first = true;
347+
for (auto s : missing) {
348+
if (first) first = false;
349+
else tail += ',';
350+
tail += toupper(s);
334351
}
335352
// special cases:
336353
// N...add line number
337-
if (strchr(required,'n') || strchr(required,'N')) {
338-
STORE("n",(double) cblock->saved_line_number);
354+
if (required.find_first_of('n') != std::string::npos || required.find_first_of('N') != std::string::npos) {
355+
STORE("n", cblock->saved_line_number);
339356
}
340357

341358
// >...require positive feed
342-
if (strchr(required,'>')) {
343-
if (settings->feed_rate > 0.0) {
344-
STORE("f",settings->feed_rate);
345-
} else {
346-
rtapi_strxcat(tail,"F>0,");
347-
errored = true;
348-
}
359+
if (required.find_first_of('>') != std::string::npos) {
360+
if (settings->feed_rate > 0.0) {
361+
STORE("f", settings->feed_rate);
362+
}
363+
else {
364+
tail += "F>0,";
365+
errored = true;
366+
}
349367
}
368+
350369
// ^...require positive speed
351-
//FIXME: How do we decide which spindle they want to use? (andypugh 17/7/16)
352-
if (strchr(required,'^')) {
353-
if (settings->speed[0] > 0.0) {
354-
STORE("s",settings->speed[0]);
355-
} else {
356-
rtapi_strxcat(tail,"S>0,");
357-
errored = true;
358-
}
370+
// FIXME: How do we decide which spindle they want to use? (andypugh 17/7/16)
371+
if (required.find_first_of('^') != std::string::npos) {
372+
if (settings->speed[0] > 0.0) {
373+
STORE("s", settings->speed[0]);
374+
}
375+
else {
376+
tail += "S>0,";
377+
errored = true;
378+
}
359379
}
360380

361381
if (errored) {
362-
ERS("user-defined %s:%s",
363-
code, tail);
382+
ERS("user-defined %s:%s", code, tail.c_str());
364383
}
365384
return INTERP_OK;
366385
}

0 commit comments

Comments
 (0)