From 6d28aa35411124ed43248cc9972a744ab281d267 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Wed, 1 Feb 2017 11:19:55 +0100 Subject: [PATCH] - do not use strtol for parsing critical values that can get large. This function will truncate everything that is larger than LONG_MAX or smaller than LONG_MIN to fit into a long variable, but longs are 32 bit on Windows and 64 bit elsewhere, so to ensure consistency and the ability to parse larger values better use strtoll which does not truncate 32 bit values. --- src/c_cmds.cpp | 4 ++-- src/c_cvars.cpp | 8 ++++---- src/c_dispatch.cpp | 2 +- src/d_dehacked.cpp | 8 ++------ src/fragglescript/t_cmd.cpp | 2 +- src/g_level.cpp | 2 +- src/gl/renderer/gl_lightdata.cpp | 2 +- src/menu/menudef.cpp | 2 +- src/menu/videomenu.cpp | 4 ++-- src/p_3dfloors.cpp | 7 ++++++- src/p_acs.cpp | 4 ++-- src/p_setup.cpp | 2 +- src/p_states.cpp | 2 +- src/parsecontext.cpp | 4 ++-- src/r_defs.h | 2 +- src/sc_man.cpp | 8 ++++---- src/scripting/decorate/olddecorations.cpp | 2 +- src/scripting/zscript/ast.cpp | 11 +++++++++++ src/win32/i_crash.cpp | 2 +- src/zstring.cpp | 4 ++-- 20 files changed, 47 insertions(+), 35 deletions(-) diff --git a/src/c_cmds.cpp b/src/c_cmds.cpp index 46ab072f21..7428fceea2 100644 --- a/src/c_cmds.cpp +++ b/src/c_cmds.cpp @@ -1190,7 +1190,7 @@ static void PrintSecretString(const char *string, bool thislevel) { if (string[1] == 'S' || string[1] == 's') { - auto secnum = strtoul(string+2, (char**)&string, 10); + auto secnum = (unsigned)strtoull(string+2, (char**)&string, 10); if (*string == ';') string++; if (thislevel && secnum < level.sectors.Size()) { @@ -1201,7 +1201,7 @@ static void PrintSecretString(const char *string, bool thislevel) } else if (string[1] == 'T' || string[1] == 't') { - long tid = strtol(string+2, (char**)&string, 10); + long tid = (long)strtoll(string+2, (char**)&string, 10); if (*string == ';') string++; FActorIterator it(tid); AActor *actor; diff --git a/src/c_cvars.cpp b/src/c_cvars.cpp index 11ebf55748..82be8663e0 100644 --- a/src/c_cvars.cpp +++ b/src/c_cvars.cpp @@ -200,7 +200,7 @@ bool FBaseCVar::ToBool (UCVarValue value, ECVarType type) else if (stricmp (value.String, "false") == 0) return false; else - return !!strtol (value.String, NULL, 0); + return !!strtoll (value.String, NULL, 0); case CVAR_GUID: return false; @@ -233,7 +233,7 @@ int FBaseCVar::ToInt (UCVarValue value, ECVarType type) else if (stricmp (value.String, "false") == 0) res = 0; else - res = strtol (value.String, NULL, 0); + res = (int)strtoll (value.String, NULL, 0); break; } case CVAR_GUID: res = 0; break; @@ -458,7 +458,7 @@ UCVarValue FBaseCVar::FromString (const char *value, ECVarType type) else if (stricmp (value, "false") == 0) ret.Bool = false; else - ret.Bool = strtol (value, NULL, 0) != 0; + ret.Bool = strtoll (value, NULL, 0) != 0; break; case CVAR_Int: @@ -467,7 +467,7 @@ UCVarValue FBaseCVar::FromString (const char *value, ECVarType type) else if (stricmp (value, "false") == 0) ret.Int = 0; else - ret.Int = strtol (value, NULL, 0); + ret.Int = (int)strtoll (value, NULL, 0); break; case CVAR_Float: diff --git a/src/c_dispatch.cpp b/src/c_dispatch.cpp index 296697ee76..52ea3d63ad 100644 --- a/src/c_dispatch.cpp +++ b/src/c_dispatch.cpp @@ -705,7 +705,7 @@ void AddCommandString (char *cmd, int keynum) if (cmd[4] == ' ') { - tics = strtol (cmd + 5, NULL, 0); + tics = (int)strtoll (cmd + 5, NULL, 0); } else { diff --git a/src/d_dehacked.cpp b/src/d_dehacked.cpp index 312333676b..7fecec550f 100644 --- a/src/d_dehacked.cpp +++ b/src/d_dehacked.cpp @@ -885,7 +885,7 @@ static int PatchThing (int thingy) while ((result = GetLine ()) == 1) { char *endptr; - unsigned long val = strtoul (Line2, &endptr, 10); + unsigned long val = (unsigned long)strtoull (Line2, &endptr, 10); size_t linelen = strlen (Line1); if (linelen == 10 && stricmp (Line1, "Hit points") == 0) @@ -1064,11 +1064,7 @@ static int PatchThing (int thingy) { if (IsNum (strval)) { - // I have no idea why everyone insists on using strtol here even though it fails - // dismally if a value is parsed where the highest bit it set. Do people really - // use negative values here? Let's better be safe and check both. - if (strchr(strval, '-')) value[0] |= (unsigned long)strtol(strval, NULL, 10); - else value[0] |= (unsigned long)strtoul(strval, NULL, 10); + value[0] |= (unsigned long)strtoll(strval, NULL, 10); vchanged[0] = true; } else diff --git a/src/fragglescript/t_cmd.cpp b/src/fragglescript/t_cmd.cpp index 6917400243..5101d00347 100644 --- a/src/fragglescript/t_cmd.cpp +++ b/src/fragglescript/t_cmd.cpp @@ -192,7 +192,7 @@ void FS_EmulateCmd(char * string) else if (sc.Compare("gr_fogcolor")) { sc.MustGetString(); - level.fadeto = strtol(sc.String, NULL, 16); + level.fadeto = (uint32_t)strtoull(sc.String, NULL, 16); } else diff --git a/src/g_level.cpp b/src/g_level.cpp index a71b481c93..28d0d64b21 100644 --- a/src/g_level.cpp +++ b/src/g_level.cpp @@ -1099,7 +1099,7 @@ void G_WorldDone (void) if (strncmp (nextlevel, "enDSeQ", 6) == 0) { - FName endsequence = ENamedName(strtol(nextlevel.GetChars()+6, NULL, 16)); + FName endsequence = ENamedName(strtoll(nextlevel.GetChars()+6, NULL, 16)); // Strife needs a special case here to choose between good and sad ending. Bad is handled elsewhere. if (endsequence == NAME_Inter_Strife) { diff --git a/src/gl/renderer/gl_lightdata.cpp b/src/gl/renderer/gl_lightdata.cpp index ae7f2a80dc..b4912f825c 100644 --- a/src/gl/renderer/gl_lightdata.cpp +++ b/src/gl/renderer/gl_lightdata.cpp @@ -552,7 +552,7 @@ CCMD(skyfog) { if (argv.argc()>1) { - skyfog=strtol(argv[1],NULL,0); + skyfog = MAX(0, (int)strtoull(argv[1], NULL, 0)); } } diff --git a/src/menu/menudef.cpp b/src/menu/menudef.cpp index d09cb40b6e..f684a8babd 100644 --- a/src/menu/menudef.cpp +++ b/src/menu/menudef.cpp @@ -666,7 +666,7 @@ static EColorRange ParseOptionColor(FScanner &sc, FOptionMenuDescriptor *desc) cr = V_FindFontColor(sc.String); if (cr == CR_UNTRANSLATED && !sc.Compare("untranslated") && isdigit(sc.String[0])) { - if (strtol(sc.String, NULL, 0)) cr = OptionSettings.mFontColorHeader; + if (strtoll(sc.String, NULL, 0)) cr = OptionSettings.mFontColorHeader; } } return cr; diff --git a/src/menu/videomenu.cpp b/src/menu/videomenu.cpp index d8edee8f1e..29192a60bc 100644 --- a/src/menu/videomenu.cpp +++ b/src/menu/videomenu.cpp @@ -366,8 +366,8 @@ static bool GetSelectedSize (int *width, int *height) char *breakpt; if (it->GetString(FOptionMenuScreenResolutionLine::SRL_INDEX+hsel, buffer, sizeof(buffer))) { - *width = strtol (buffer, &breakpt, 10); - *height = strtol (breakpt+1, NULL, 10); + *width = (int)strtoll (buffer, &breakpt, 10); + *height = (int)strtoll (breakpt+1, NULL, 10); return true; } } diff --git a/src/p_3dfloors.cpp b/src/p_3dfloors.cpp index 77e34adbab..f367c46377 100644 --- a/src/p_3dfloors.cpp +++ b/src/p_3dfloors.cpp @@ -982,7 +982,12 @@ CCMD (dump3df) { if (argv.argc() > 1) { - int sec = strtol(argv[1], NULL, 10); + int sec = (int)strtoll(argv[1], NULL, 10); + if ((unsigned)sec >= level.sectors.Size()) + { + Printf("Sector %d does not exist.\n", sec); + return; + } sector_t *sector = &level.sectors[sec]; TArray & ffloors=sector->e->XFloor.ffloors; diff --git a/src/p_acs.cpp b/src/p_acs.cpp index e346579c4c..8e7ba76881 100644 --- a/src/p_acs.cpp +++ b/src/p_acs.cpp @@ -1086,12 +1086,12 @@ static void ReadArrayVars (FSerializer &file, FWorldGlobalArray *vars, size_t co const char *arraykey; while ((arraykey = file.GetKey())) { - int i = (int)strtol(arraykey, nullptr, 10); + int i = (int)strtoll(arraykey, nullptr, 10); if (file.BeginObject(nullptr)) { while ((arraykey = file.GetKey())) { - int k = (int)strtol(arraykey, nullptr, 10); + int k = (int)strtoll(arraykey, nullptr, 10); int val; file(nullptr, val); vars[i].Insert(k, val); diff --git a/src/p_setup.cpp b/src/p_setup.cpp index a4e4ef1b9a..18b3966c70 100644 --- a/src/p_setup.cpp +++ b/src/p_setup.cpp @@ -2540,7 +2540,7 @@ void P_ProcessSideTextures(bool checktranmap, side_t *sd, sector_t *sec, intmaps case Sector_Set3DFloor: if (msd->toptexture[0]=='#') { - sd->SetTexture(side_t::top, FNullTextureID() +(-strtol(&msd->toptexture[1], NULL, 10))); // store the alpha as a negative texture index + sd->SetTexture(side_t::top, FNullTextureID() +(int)(-strtoll(&msd->toptexture[1], NULL, 10))); // store the alpha as a negative texture index // This will be sorted out by the 3D-floor code later. } else diff --git a/src/p_states.cpp b/src/p_states.cpp index 09591d70f7..221ff6cedf 100644 --- a/src/p_states.cpp +++ b/src/p_states.cpp @@ -731,7 +731,7 @@ FState *FStateDefinitions::ResolveGotoLabel (AActor *actor, PClassActor *mytype, *pt = '\0'; offset = pt + 1; } - v = offset ? strtol (offset, NULL, 0) : 0; + v = offset ? (int)strtoll (offset, NULL, 0) : 0; // Get the state's address. if (type == mytype) diff --git a/src/parsecontext.cpp b/src/parsecontext.cpp index 2cfaafa295..cdc28c15a8 100644 --- a/src/parsecontext.cpp +++ b/src/parsecontext.cpp @@ -103,7 +103,7 @@ loop: c = *sourcep++; if (c == 'x' || c == 'X') { - yylval->val = (int)strtol(sourcep, &sourcep, 16); + yylval->val = (int)strtoll(sourcep, &sourcep, 16); return TokenTrans[NUM]; } else @@ -114,7 +114,7 @@ loop: char *endp; sourcep--; - yylval->val = (int)strtol(sourcep, &endp, 10); + yylval->val = (int)strtoll(sourcep, &endp, 10); if (*endp == '.') { // It's a float diff --git a/src/r_defs.h b/src/r_defs.h index b888864b4c..08c27bfd39 100644 --- a/src/r_defs.h +++ b/src/r_defs.h @@ -239,7 +239,7 @@ struct FUDMFKey FUDMFKey& operator =(const FString &val) { Type = UDMF_String; - IntVal = strtol(val.GetChars(), NULL, 0); + IntVal = (int)strtoll(val.GetChars(), NULL, 0); FloatVal = strtod(val.GetChars(), NULL); StringVal = val; return *this; diff --git a/src/sc_man.cpp b/src/sc_man.cpp index 5f944735a7..f1ee06693c 100644 --- a/src/sc_man.cpp +++ b/src/sc_man.cpp @@ -554,12 +554,12 @@ bool FScanner::GetToken () String[StringLen - 2] == 'u' || String[StringLen - 2] == 'U') { TokenType = TK_UIntConst; - Number = strtoul(String, &stopper, 0); + Number = strtoull(String, &stopper, 0); Float = (unsigned)Number; } else { - Number = strtol(String, &stopper, 0); + Number = (int)strtoll(String, &stopper, 0); Float = Number; } } @@ -660,7 +660,7 @@ bool FScanner::GetNumber () } else { - Number = strtol (String, &stopper, 0); + Number = (int)strtoll (String, &stopper, 0); if (*stopper != 0) { ScriptError ("SC_GetNumber: Bad numeric constant \"%s\".", String); @@ -715,7 +715,7 @@ bool FScanner::CheckNumber () } else { - Number = strtol (String, &stopper, 0); + Number = (int)strtoll (String, &stopper, 0); if (*stopper != 0) { UnGet(); diff --git a/src/scripting/decorate/olddecorations.cpp b/src/scripting/decorate/olddecorations.cpp index 1eeed5796e..5861138d66 100644 --- a/src/scripting/decorate/olddecorations.cpp +++ b/src/scripting/decorate/olddecorations.cpp @@ -635,7 +635,7 @@ static void ParseSpriteFrames (PClassActor *info, TArray &states, TArray char *stop; *colon = 0; - rate = strtol (token, &stop, 10); + rate = (int)strtoll (token, &stop, 10); if (stop == token || rate < 1 || rate > 65534) { sc.ScriptError ("Rates must be in the range [0,65534]"); diff --git a/src/scripting/zscript/ast.cpp b/src/scripting/zscript/ast.cpp index 586601890e..c4c8a12a51 100644 --- a/src/scripting/zscript/ast.cpp +++ b/src/scripting/zscript/ast.cpp @@ -346,6 +346,16 @@ static void PrintProperty(FLispString &out, ZCC_TreeNode *node) out.Close(); } +static void PrintStaticArrayState(FLispString &out, ZCC_TreeNode *node) +{ + auto *snode = (ZCC_StaticArrayStatement *)node; + out.Break(); + out.Open("static-array"); + out.AddName(snode->Id); + PrintNodes(out, snode->Values, false, true); + out.Close(); +} + static void PrintEnum(FLispString &out, ZCC_TreeNode *node) { ZCC_Enum *enode = (ZCC_Enum *)node; @@ -944,6 +954,7 @@ void (* const TreeNodePrinter[NUM_AST_NODE_TYPES])(FLispString &, ZCC_TreeNode * PrintVectorInitializer, PrintDeclFlags, PrintExprClassCast, + PrintStaticArrayState, PrintProperty, }; diff --git a/src/win32/i_crash.cpp b/src/win32/i_crash.cpp index 4a0a1b1606..5c1678c5c2 100644 --- a/src/win32/i_crash.cpp +++ b/src/win32/i_crash.cpp @@ -2731,7 +2731,7 @@ static bool ReadResponse (HWND hDlg, char *header, SOCKET sock, char *buf, int b char *lenhead = strstr (header, "content-length: "); if (lenhead != 0) { - len = strtol (lenhead + 16, NULL, 10); + len = (int)strtoll (lenhead + 16, NULL, 10); if (file != INVALID_HANDLE_VALUE) { ShowWindow (GetDlgItem (hDlg, IDC_BOINGPROGRESS), SW_SHOW); diff --git a/src/zstring.cpp b/src/zstring.cpp index d17348bd31..665b9312ee 100644 --- a/src/zstring.cpp +++ b/src/zstring.cpp @@ -1104,12 +1104,12 @@ digits = [0-9]; long FString::ToLong (int base) const { - return strtol (Chars, NULL, base); + return (long)strtoll (Chars, NULL, base); } unsigned long FString::ToULong (int base) const { - return strtoul (Chars, NULL, base); + return (unsigned long)strtoull (Chars, NULL, base); } double FString::ToDouble () const