rxi / aria

A tiny, embeddable lisp-shaped language implemented in C89
MIT License
167 stars 18 forks source link

Use secure versions of sprintf, fopen, and wsprintf #5

Open jason-c-daniels opened 7 years ago

jason-c-daniels commented 7 years ago

Use Case: Consuming applications need to be as secure as possible. They can be no more secure than their least secure component. By securing the features using these functions you improve the security of client applications.

Estimated Impact:

Cons: There will be some extra coding will need to be done around fopen_s as it returns an error code value. Pros:

  1. using fopen_s this will more easily allow the code to give more meaningful interpretations of various file system errors, vs, just a simple success/failure on file access.
  2. Doing the rest give peace of mind to consumers.
  3. Some compiler settings will prevent compilation if using the insecure alternatives. Making these changes will allow people to retain stringent security checks in their compilers without special casing the code in aria.c.
  4. I can submit these changes to you if you would like. (I'm 99% done. I've not properly used the return code from fopen_s yet)
rxi commented 7 years ago

Regarding sprintf -- this function is only used in 3 places:

Aria tries to keep strictly to C89. If a specific compiler refuses to compile because sprintf is being used I would be willing to add a an ifdef-wrapped define (or something to this effect) to pacify these warnings/errors, but to keep in line with the project's goal of minimalism and C89 compatibility I wouldn't be willing to replace these functions with the C99/11 alternatives.

jason-c-daniels commented 7 years ago

I'll work up an #ifdef macro to do this and submit a pull request, since VS2015 with security checking is what's barking about this. I pulled aria into it for memory profiling for potential leak detection. (I found none)