#8409 closed patch
replace vsprintf usage with vsnprintf
| Reported by: | SF/khalek | Owned by: | SF/khalek |
|---|---|---|---|
| Priority: | normal | Component: | --Other-- |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
vsprintf is not a safe string function, vsnprintf should be used.
The only problem is the apparent lack of implementation on palmos and perhaps other platforms?
Local implementations should be created for these platforms.
Ticket imported from: #1168900. Ticket imported from: patches/514.
Attachments (1)
Change History (7)
by , 19 years ago
| Attachment: | vsnprintf.diff added |
|---|
comment:1 by , 19 years ago
Indeed, vsnprintf is not very portable, which is the main reason I didn't use it so far (I was considering switching to it at some point).
Do we really want to provide our own vsnprintf implementation (i.e. the full code for a full blown printf clone), just so we can avoid some hypothetical overflows in a non-security relevant area? hmmm
comment:2 by , 19 years ago
People are likely to look at the code and use it or something like it in a security sensitive area. Teaching people good programming practice is important...
A local implementation would only be used if there was not an existing implementation, the palmos backend already does this for a lot of things because Palm only seem to implement most common functions in a version of PalmOS that isn't actually available on real hardware yet.
comment:3 by , 19 years ago
I think that justification is nonsense -- people who look at our code and thing it is an example of how to code for a security sensitive area are nuts anyway. E.g. there are zillions of places where we use snprintf. Or read from a file and work with its content w/o verifying it properly. Etc. etc. -- if you install ScummVM in a way that gives it too many rights, you are doomed either way.
Anyway, apparently you just checked this patch in anyway, so I assume it can be closed. If we get troubles with porters, we can still revert it.
comment:4 by , 19 years ago
Given we were already using vsprintf I don't think asking for vsnprintf is that much of a stretch.
Peope get code examples from all over the place and sometimes things as non critical as ScummVM could become critical. It would be nice to be able to point people at the code as an example of good programming practice. I am of the opinion it is necessary to not dismiss things like using safe string functions because a context is assumed for the code.
Once every few weeks someone comes along wanting to adapt ScummVM for some other purpose. The use of these bad string functions can hopefully be limited by having fewer examples of their use.
I suspect you mean sprintf not snprintf. There are many places where strcat and strcpy are used instead of safe things like strlcat and strlcpy in the code as it stands now as well.
comment:5 by , 19 years ago
| Owner: | set to |
|---|---|
| Status: | new → closed |
comment:6 by , 5 years ago
| Component: | → --Other-- |
|---|
vsnprintf diff