Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed possible buffer overflow + various Win32/MSVC build fixes + Win32 Unicode support #4

Merged
merged 5 commits into from
Dec 16, 2017

Conversation

lordmulder
Copy link
Contributor

…when not connected directly to a terminal. This is required so that progress updates don't get stuck in pipes.
…cated with fixed size, but a string of *unbounded* size (because it contains a user-supplied the file name) was written into that buffer via sprintf().
@knik0
Copy link
Owner

knik0 commented Dec 16, 2017

It looks like faad_fopen is undefined, probably only defined in Windows.

@lordmulder
Copy link
Contributor Author

lordmulder commented Dec 16, 2017

Thanks for reply.

It is defined in my unicode_support.h file, and implemented in unicode_support.c:
435ec28#diff-68ac2deccaccc19956ff4720b6566ab5

Note: This is pretty much the same code that I developed for and contributed to Opus-Tools:
https://git.xiph.org/?p=opus-tools.git;a=blob;f=win32/unicode_support.c;h=f057ac04077667bce3502c4b7f2d1b9c6951baa5;hb=HEAD


[EDIT]

unicode_support.h contains #defines that map faad_fopen() to fopen() on POSIX platforms.

@knik0
Copy link
Owner

knik0 commented Dec 16, 2017

Is is save to include it on all platforms, in the patch it's only included in windows.

@lordmulder
Copy link
Contributor Author

lordmulder commented Dec 16, 2017

Is is save to include it on all platforms, in the patch it's only included in windows.

Yes, should be safe. Including it only on Win32 was my mistake, sorry.


Maybe the guard in unicode_support.h should be changed from:

#ifdef _WIN32
...
#else
...
#endif

...to:

#if defined WIN32 || defined _WIN32 || defined WIN64 || defined _WIN64
...
#else
...
#endif

Also, the function prototypes defined above should be inside the guard, I suppose.

@lordmulder
Copy link
Contributor Author

I have updated the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants