Largo / ocran

Turn ruby files into .exe files on windows (supported safe fork of ocran)
MIT License
66 stars 6 forks source link

Refactoring of stub.c #11

Open shinokaro opened 9 months ago

shinokaro commented 9 months ago

@Largo

The implementation of stub.c had security issues, such as the use of strcat with fixed-length buffers, and the code complexity made it difficult to review partial fixes due to the mixture of TCHAR and char types.

I spent a month thoroughly understanding stub.c and its design, and I have been refactoring stub.exe to make it secure and the code more readable. The refactoring follows these guidelines:

This effort is still ongoing, but the implementation of stub has become safer than before. However, several more changes are needed to complete the secure implementation.

stub.c compiles successfully with Mingw32 and Mingw64 on Windows. Please let me know if there are any compilation issues with Wine.

Largo commented 9 months ago

Thank you so much. I looked into it and it looks good! I'm preparing a release right now.

shinokaro commented 8 months ago

The source code of the stub has now been made readable by humans.

The current implementation standardizes file path handling to UTF-8 and uses a variable-length buffer for path concatenation, eliminating the dependency on MAX_PATH and the possibility of buffer overruns. Additionally, security risks associated with script execution have been resolved. The stub cannot execute any scripts located externally.

However, security risks that have existed since before the fork from OCRA have not yet been addressed. In the next step, we will resolve these remaining security risks.

shinokaro commented 8 months ago

The commit at https://github.com/Largo/ocran/commit/72c376db577620f25f35246f18f333476c563141 resolved a potential security issue, preventing files from being written outside the designated directory and execution of external scripts via 'stub'.

shinokaro commented 8 months ago

The refactoring of stub.c has been completed in the commit https://github.com/Largo/ocran/commit/a7b8347ba95879b2722b4c4415d404de2f711ed2. The clear security issues in the implementation have been resolved. However, since security standards in actual operation vary by organization, I believe there are still areas for improvement based on external feedback.

The next step for stub is to handle paths with multibyte character strings. Users outside English-speaking countries use characters from their own languages in their usernames, which appear in the temporary paths. Additionally, I believe there is a need to replace WIN32API calls with standard C functions.