From 666c856457ebda1a4261d0da95c11ecefc04708f Mon Sep 17 00:00:00 2001 From: frosch Date: Sun, 3 Jan 2010 19:29:56 +0000 Subject: (svn r18699) -Fix [FS#PlanetAndy]: GRF parameters were not properly initialised to zero, and not always checked for valid range. --- src/newgrf.cpp | 25 ++++++++++++++++--------- src/newgrf.h | 9 +++++++++ src/newgrf_spritegroup.cpp | 4 ++-- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/newgrf.cpp b/src/newgrf.cpp index 7d21ba60b..0b6130a72 100644 --- a/src/newgrf.cpp +++ b/src/newgrf.cpp @@ -3863,7 +3863,7 @@ static uint32 GetParamVal(byte param, uint32 *cond_val) default: /* GRF Parameter */ - if (param < 0x80) return _cur_grffile->param[param]; + if (param < 0x80) return _cur_grffile->GetParam(param); /* In-game variable. */ grfmsg(1, "Unsupported in-game variable 0x%02X", param); @@ -4380,7 +4380,7 @@ static void GRFLoadError(byte *buf, size_t len) uint i = 0; for (; i < 2 && len > 0; i++) { uint param_number = grf_load_byte(&buf); - error->param_value[i] = (param_number < _cur_grffile->param_end ? _cur_grffile->param[param_number] : 0); + error->param_value[i] = _cur_grffile->GetParam(param_number); len--; } error->num_params = i; @@ -4493,11 +4493,11 @@ static uint32 PerformGRM(uint32 *grm, uint16 num_ids, uint16 count, uint8 op, ui if (op == 6) { /* Return GRFID of set that reserved ID */ - return grm[_cur_grffile->param[target]]; + return grm[_cur_grffile->GetParam(target)]; } /* With an operation of 2 or 3, we want to reserve a specific block of IDs */ - if (op == 2 || op == 3) start = _cur_grffile->param[target]; + if (op == 2 || op == 3) start = _cur_grffile->GetParam(target); for (uint i = start; i < num_ids; i++) { if (grm[i] == 0) { @@ -4631,7 +4631,7 @@ static void ParamSet(byte *buf, size_t len) switch (op) { case 2: case 3: - src1 = _cur_grffile->param[target]; + src1 = _cur_grffile->GetParam(target); break; default: @@ -4680,10 +4680,10 @@ static void ParamSet(byte *buf, size_t len) /* Disable the read GRF if it is a static NewGRF. */ DisableStaticNewGRFInfluencingNonStaticNewGRFs(c); src1 = 0; - } else if (file == NULL || src1 >= file->param_end || (c != NULL && c->status == GCS_DISABLED)) { + } else if (file == NULL || (c != NULL && c->status == GCS_DISABLED)) { src1 = 0; } else { - src1 = file->param[src1]; + src1 = file->GetParam(src1); } } } else { @@ -4829,6 +4829,7 @@ static void ParamSet(byte *buf, size_t len) default: if (target < 0x80) { _cur_grffile->param[target] = res; + /* param is zeroed by default */ if (target + 1U > _cur_grffile->param_end) _cur_grffile->param_end = target + 1; } else { grfmsg(7, "ParamSet: Skipping unknown target 0x%02X", target); @@ -5700,10 +5701,16 @@ static void InitNewGRFFile(const GRFConfig *config, int sprite_offset) newfile->price_base_multipliers[i] = INVALID_PRICE_MODIFIER; } - /* Copy the initial parameter list */ + /* Copy the initial parameter list + * 'Uninitialised' parameters are zeroed as that is their default value when dynamically creating them. */ assert_compile(lengthof(newfile->param) == lengthof(config->param) && lengthof(config->param) == 0x80); + memset(newfile->param, 0, sizeof(newfile->param)); + + assert(config->num_params <= lengthof(config->param)); newfile->param_end = config->num_params; - memcpy(newfile->param, config->param, sizeof(newfile->param)); + if (newfile->param_end > 0) { + MemCpyT(newfile->param, config->param, newfile->param_end); + } *_grf_files.Append() = _cur_grffile = newfile; } diff --git a/src/newgrf.h b/src/newgrf.h index ea9b890a0..64b8f9172 100644 --- a/src/newgrf.h +++ b/src/newgrf.h @@ -118,6 +118,15 @@ struct GRFFile { uint32 grf_features; ///< Bitset of GrfSpecFeature the grf uses PriceMultipliers price_base_multipliers; ///< Price base multipliers as set by the grf. + + /** Get GRF Parameter with range checking */ + uint32 GetParam(uint number) const + { + /* Note: We implicitly test for number < lengthof(this->param) and return 0 for invalid parameters. + * In fact this is the more important test, as param is zeroed anyway. */ + assert(this->param_end <= lengthof(this->param)); + return (number < this->param_end) ? this->param[number] : 0; + } }; enum ShoreReplacement { diff --git a/src/newgrf_spritegroup.cpp b/src/newgrf_spritegroup.cpp index 74cba113b..348f8ebf0 100644 --- a/src/newgrf_spritegroup.cpp +++ b/src/newgrf_spritegroup.cpp @@ -62,8 +62,8 @@ static inline uint32 GetVariable(const ResolverObject *object, byte variable, by case 0x7D: return _temp_store.Get(parameter); case 0x7F: - if (object == NULL || object->grffile == NULL || parameter >= object->grffile->param_end) return 0; - return object->grffile->param[parameter]; + if (object == NULL || object->grffile == NULL) return 0; + return object->grffile->GetParam(parameter); /* Not a common variable, so evalute the feature specific variables */ default: return object->GetVariable(object, variable, parameter, available); -- cgit v1.2.3-54-g00ecf