Skip to content
Snippets Groups Projects
Commit 71ba1785 authored by rswindell's avatar rswindell
Browse files

Fix multiple potential buffer overflows in external() (for Windows) with

"overly long" cmdlines.
Also fixed a bug (for Windows) where external() would return 0 (success) even
when CreateProcess() fails - must re-restore the "last_error" value before
returning.
parent 973f2000
No related branches found
No related tags found
No related merge requests found
...@@ -433,9 +433,9 @@ int sbbs_t::external(const char* cmdline, long mode, const char* startup_dir) ...@@ -433,9 +433,9 @@ int sbbs_t::external(const char* cmdline, long mode, const char* startup_dir)
if(startup_dir && cmdline[1]!=':' && cmdline[0]!='/' if(startup_dir && cmdline[1]!=':' && cmdline[0]!='/'
&& cmdline[0]!='\\' && cmdline[0]!='.') && cmdline[0]!='\\' && cmdline[0]!='.')
sprintf(fullcmdline, "%s%s%s", comspec_str, startup_dir, cmdline); SAFEPRINTF3(fullcmdline, "%s%s%s", comspec_str, startup_dir, cmdline);
else else
sprintf(fullcmdline, "%s%s", comspec_str, cmdline); SAFEPRINTF2(fullcmdline, "%s%s", comspec_str, cmdline);
SAFECOPY(realcmdline, fullcmdline); // for errormsg if failed to execute SAFECOPY(realcmdline, fullcmdline); // for errormsg if failed to execute
...@@ -542,7 +542,7 @@ int sbbs_t::external(const char* cmdline, long mode, const char* startup_dir) ...@@ -542,7 +542,7 @@ int sbbs_t::external(const char* cmdline, long mode, const char* startup_dir)
fprintf(fp, "YEAR=%u\n",1900+tm.tm_year); fprintf(fp, "YEAR=%u\n",1900+tm.tm_year);
fclose(fp); fclose(fp);
sprintf(fullcmdline, "%sDOSXTRN.EXE %s", cfg.exec_dir, path); SAFEPRINTF2(fullcmdline, "%sDOSXTRN.EXE %s", cfg.exec_dir, path);
if(!(mode&EX_OFFLINE) && nt) { // Windows NT/2000 if(!(mode&EX_OFFLINE) && nt) { // Windows NT/2000
i=SBBSEXEC_MODE_FOSSIL; i=SBBSEXEC_MODE_FOSSIL;
...@@ -655,7 +655,7 @@ int sbbs_t::external(const char* cmdline, long mode, const char* startup_dir) ...@@ -655,7 +655,7 @@ int sbbs_t::external(const char* cmdline, long mode, const char* startup_dir)
if(mode&EX_OFFLINE) if(mode&EX_OFFLINE)
startup_info.lpTitle=NULL; startup_info.lpTitle=NULL;
else { else {
sprintf(title,"%s running %s on node %d" SAFEPRINTF3(title,"%s running %s on node %d"
,useron.number ? useron.alias : "Event" ,useron.number ? useron.alias : "Event"
,realcmdline ,realcmdline
,cfg.node_num); ,cfg.node_num);
...@@ -737,6 +737,7 @@ int sbbs_t::external(const char* cmdline, long mode, const char* startup_dir) ...@@ -737,6 +737,7 @@ int sbbs_t::external(const char* cmdline, long mode, const char* startup_dir)
} }
SetLastError(last_error); /* Restore LastError */ SetLastError(last_error); /* Restore LastError */
errormsg(WHERE, ERR_EXEC, realcmdline, mode); errormsg(WHERE, ERR_EXEC, realcmdline, mode);
SetLastError(last_error); /* Restore LastError */
return(GetLastError()); return(GetLastError());
} }
...@@ -2080,7 +2081,8 @@ char* sbbs_t::cmdstr(const char *instr, const char *fpath, const char *fspec, ch ...@@ -2080,7 +2081,8 @@ char* sbbs_t::cmdstr(const char *instr, const char *fpath, const char *fspec, ch
else else
cmd=outstr; cmd=outstr;
len=strlen(instr); len=strlen(instr);
for(i=j=0; i<len && j < (int)sizeof(cmdstr_output)-1; i++) { int maxlen = (int)sizeof(cmdstr_output) - 1;
for(i=j=0; i<len && j < maxlen; i++) {
if(instr[i]=='%') { if(instr[i]=='%') {
i++; i++;
cmd[j]=0; cmd[j]=0;
...@@ -2143,7 +2145,7 @@ char* sbbs_t::cmdstr(const char *instr, const char *fpath, const char *fspec, ch ...@@ -2143,7 +2145,7 @@ char* sbbs_t::cmdstr(const char *instr, const char *fpath, const char *fspec, ch
strcat(cmd,ultoa(rows,str,10)); strcat(cmd,ultoa(rows,str,10));
break; break;
case 'S': /* File Spec (or Baja command str) */ case 'S': /* File Spec (or Baja command str) */
strcat(cmd,fspec); strncat(cmd, fspec, maxlen - j);
break; break;
case 'T': /* Time left in seconds */ case 'T': /* Time left in seconds */
gettimeleft(); gettimeleft();
...@@ -2244,7 +2246,8 @@ char* DLLCALL cmdstr(scfg_t* cfg, user_t* user, const char* instr, const char* f ...@@ -2244,7 +2246,8 @@ char* DLLCALL cmdstr(scfg_t* cfg, user_t* user, const char* instr, const char* f
if(cmd==NULL) cmd=buf; if(cmd==NULL) cmd=buf;
len=strlen(instr); len=strlen(instr);
for(i=j=0; i<len && j < (int)sizeof(buf)-1; i++) { int maxlen = (int)sizeof(buf) - 1;
for(i=j=0; i<len && j < maxlen; i++) {
if(instr[i]=='%') { if(instr[i]=='%') {
i++; i++;
cmd[j]=0; cmd[j]=0;
...@@ -2308,7 +2311,7 @@ char* DLLCALL cmdstr(scfg_t* cfg, user_t* user, const char* instr, const char* f ...@@ -2308,7 +2311,7 @@ char* DLLCALL cmdstr(scfg_t* cfg, user_t* user, const char* instr, const char* f
strcat(cmd,ultoa(user->rows,str,10)); strcat(cmd,ultoa(user->rows,str,10));
break; break;
case 'S': /* File Spec */ case 'S': /* File Spec */
strcat(cmd,fspec); strncat(cmd, fspec, maxlen - j);
break; break;
case 'T': /* Time left in seconds */ case 'T': /* Time left in seconds */
break; break;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment