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

[core] GetFileLength() overflows with file larger than 2GB #3262

Closed
michelelorusso05 opened this issue Aug 25, 2023 · 9 comments
Closed

[core] GetFileLength() overflows with file larger than 2GB #3262

michelelorusso05 opened this issue Aug 25, 2023 · 9 comments

Comments

@michelelorusso05
Copy link

The IO method GetFileLength returns the size of a given file stored in an int variable. Since it's only an unsigned 32 bit number, the function works properly only with files smaller than 2147483647 bytes. Any method that operates on file sizes should return an unsigned long.

@orcmid
Copy link
Contributor

orcmid commented Aug 25, 2023

That's a problem. There is no requirement that unsigned long have a larger maximum size than unsigned int, which is also required to have the sane maximum, at least 2147483647. For more you need unsigned long long and a hefty 0xffffffffffffffffui64 minimum maximum.

These values for <limits.h> are established in ISO C11. I would not expect them to have changed since.

@michelelorusso05
Copy link
Author

Yeah, you're right. I meant that an unsigned 64 bit integer would be more suitable, and on most modern compilers an unsigned long is just that. The function should return a uint64_t.

@orcmid
Copy link
Contributor

orcmid commented Aug 26, 2023

Oh Oh. Well, I looked at GetFileLength() in rcore.c and there are many problems.

First, it opens the file named and if that fails it returns 0 (rather than EOF or something).

Secondly, it seeks to the end of the file using fseek(file, 0L, SEEK_END) without checking for any error case.

Finally it returns size from

size = (int) ftell(file);

There are three problems here. ftell returns a long, not an int. There is also some question about whether the position of the end of the file is the same as the size of the file. There is also no failure check on the ftell result.

I suppose we can confirm the answer the second question by doing an

fseek(file, 0L, SEEK_SET);

and seeing what an ftell returns after that.

For files longer than INT_MAX or LONG_MAX another solution is required.

The Microsoft <stdio.h> includes _ftelli64 which handles the situation by presumably returning an int64. That's definitely not C99 or C11, and must be considered a non-ISO extension unless something shows up in C17 or C23. I suppose that means Microsoft also has an int64 version of fseek and indeed it does. :).

@raysan5 raysan5 changed the title GetFileLength(const char* filename) overflows with file larger than 2GB [core] GetFileLength() overflows with file larger than 2GB Aug 26, 2023
@raysan5
Copy link
Owner

raysan5 commented Aug 26, 2023

@michelelorusso05 @orcmid that function was added for my convenience and I'm aware of its deficiencies.

Also, in raylib I try to avoid stdint.h types and just use the language provided ones. In any case, it can be updated to return a long instead of int or just add some notes about the limitations of this function.

Is there a better multiplatform approach to get filesize than using fseek()/ftell()?

@michelelorusso05
Copy link
Author

As far as I am aware, fseek/ftell is the most convenient way, cross-platform speaking. As for the returned type, since long can be either 32 bit or 64 bit based on the implementation, employing an unsigned long long can ensure an at least 64 bit value.

@raysan5
Copy link
Owner

raysan5 commented Aug 26, 2023

@michelelorusso05 On raylib all data-size-related variables, even the referred to file data, are int or unsigned int, changing this function to unsigned long long will break consistency with all the other functions just to cover a specific use case that probably not many users will need.

In any case, it can be checked inside and if the file is bigger than 2/4GB return 0 and TraceLog() a warning message, that's the usual mechanism used by raylib on those situations.

This issue is somewhat related to #3168.

@michelelorusso05
Copy link
Author

I guess that the function should be left as is, for the sake of consistency. However, as you said, the docs should be updated to describe this behaviour.

@orcmid
Copy link
Contributor

orcmid commented Aug 26, 2023

@raysan5

Is there a better multiplatform approach to get filesize than using fseek()/ftell()?

It depends how widely available stat is. This provides size information from the file-system directory and not by seeking to the end. There are problems for non-binary text files having to do with \n and \0x1A in some files.

There seem to be flavors that address the need for very large file sizes.
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions.

@raysan5
Copy link
Owner

raysan5 commented Aug 29, 2023

Reviewed, added better documentation and tracelog

@raysan5 raysan5 closed this as completed Aug 29, 2023
raysan5 added a commit that referenced this issue Sep 17, 2023
* Prettified a comment

* fixed broken indentation caused by another commit.
the commit renamed a bool to int and broke indentation: 233cf39

* Changed 0.001 and 0.00001 to EPSILON
This commit is untested.
I don't know what consequences this has.
Since the commits that added these numbers were before epsilon was added,
I have assumed that epsilon could replace them.

* Prettied up indentation in a few places

* removed spacing around *, standardizing it.

* I may have gotten overboard with indentation

* removed a few useless parenthesis

* Added fortran-raylib

* Fix examples/others/rlgl_standalone.c compilation issue (#3242)

* Update BINDINGS.md

* Ignore unused return value of GetCodepointNext in GetCodepointCount (#3241)

* Ignore unused return value of GetCodepointNext in GetCodepointCount

Removes the last warning from non-external libraries when compiling with
the default build configuration on x64 Linux.

* Remove unnecessary void cast in GetCodepointCount

* Fix #3246

* Revert "Fix #3246"

This reverts commit e4dcbd5.

* Fix text_unicode.c example crashing (#3250)

* Fix text_unicode.c example crashing

* Adjust the text_unicode.c example crashing fix

* tweaks

* add build.zig options for individual modules (#3254)

* Add `IsKeyPressedRepeat` (desktop only) (#3245)

Since the key pressed are handle by comparing current vs previous
state (ie frame), a special way is needed to handle key repeats.

* Reviewed `IsKeyPressedRepeat()` #3248

* Update rcore.c (#3255)

* Match CMakeOptions.txt options default values (#3258)

* Fix SetClipboardText for web (#3257)

* [Image] Validate that ImageDrawRectangleRec is drawing entirely inside the image (#3264)

* Add a function to clone a sound and share data with another sound.

* rename items based on feedback

* PR Feedback, use custom unload for sound alias, not variant of normal sound unloading

* sound_multi example

* Validate that image rect drawing is inside the image so we don't overflow a buffer

* remove files that should not have been added.

* remove changes that should not have been

* revert

* adsfasdfsdfsdf

* Add Vector3 Projecting and Rejection to Raymath (#3263)

* Update raymath.h

* formatting

* [Feature] IsKey... safety checks and more (#3256)

* [Feature] Add GetKeyRepeat

* Update rcore.c

* Simpler design, only one repeat per frame

* Update config.h

* Update rcore.c

* Add KEYBOARD_KEYS_MASK

* Update config.h

* reversions

* Update rcore.c

* Update rcore.c

* change docs

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update raylib.h

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Update rcore.c

* Fix bug where default shaders was not linking. (#3261)

* Formating review

* Add missing cmake options (#3267)

* Fix CMake extraneous -lglfw (#3266)

Closes #3265.

The problem: LIBS_PRIVATE is a list of library names (used by pkg-config), but the shared library of the same name doesn't always exist.

* Fix example/models/models_loading_gltf.c controls (#3268)

* Fix example/models/models_loading_m3d.c controls (#3269)

* Remove e from secondes (#3270)

* Fix example/audio/audio_module_player.c help instructions and small bug (#3272)

* Fix example/audio/audio_module_player.c help instructions and small bug

* Update example/audio/audio_module_player.png screenshot

* Use type name instead of valid specifier

long long --> long long int

* REVIEWED: `GetFileLength()`, added comment #3262

* Update examples/models/models_loading_gltf.png;m3d.png screenshots (#3273)

* Remove a duplicated screenshot and add missing one (#3275)

* Add examples/shaders/shaders_lightmap.c to Makefiles (#3276)

* Fix examples/others/easings_testbed.c help instructions and small tweak (#3277)

* Fix examples/shaders/shaders_texture_outline.c help instructions (#3278)

* Fix examples/shapes/shapes_collision_area.c help instructions (#3279)

* RENAMED: LoadFont*() parameter names for consistency and coherence

* Fix uninitialized thread-locals in stbi #3282 (#3283)

* REVIEWED: Added `SetTextLineSpacing()` to multiline examples

* REVIEWED: Data size type consistency between functions #3168

* Some tweaks

* Use internal default allocators, instead of user-exposed ones

* Added rudimentary SVG support. (#2738)

* Added rudimentary SVG support. Added 2 functions ImageLoadSvg and ImageLoadSvgWithSize.

* Added an example on how to use ImageLoadSvgWithSize and adjusted Makefiles accordingly.

* Added actual correct example file.

* Reviewed the code to keep the raylib coding conventions in mind.
Moved the LoadImageSvg() code into LoadImage() guarded by SUPPORT_FILEFORMAT_SVG.
Renamed LoadImageSvgWithSize() to LoadImageSvg().
Added a LoadImageSvgFromString() function to parse the loaded SVG into an actual image. This does the bulk of the work.

* Fixed typo.

---------

Co-authored-by: Ray <[email protected]>

* REVIEWED: `LoadImageSvg()`

* REVIEWED: `LoadImageSvg()`

* Add SUPPORT_FILEFORMAT_SVG to cmake (#3284)

* Fix examples/textures/textures_fog_of_war.c help instructions (#3285)

* Fix examples/textures/textures_image_rotate.c help instructions (#3286)

* Update rtextures.c

* Fix #3247

* Update config.h

* Fix #3293

* Disable UBSAN in zig builds. (#3292)

Zig debug builds automatically enable ubsan.
As the fix for #1891 had to be reverted, debug builds using zig will crash like so:

```
Illegal instruction at address 0x3237d2
raylib/src/rlgl.h:3690:91: 0x3237d2 in rlDrawVertexArrayElements (/home/rcorre/src/raylib-zig-template/raylib/src/rcore.c)
    glDrawElements(GL_TRIANGLES, count, GL_UNSIGNED_SHORT, (const unsigned short *)buffer + offset);
```

This disables UBSAN when using zig to build raylib.

* Update README.md (#3290)

specially -> especially

* Update cmake SUPPORT_FILEFORMAT_SVG default value (#3291)

* Mouse offset and scaling must be considered also on web!

* Update rcore.c

* Update Makefile : clean raygui.c & physac.c (#3296)

* Remove PLATFORM_RPI (#3232)

* Remove PLATFORM_RPI

* remove build artifacts

---------

Co-authored-by: MichaelFiber <[email protected]>
Co-authored-by: Ray <[email protected]>

* Review to avoid UBSAN complaining #1891

* added raylib-raku to bindings (#3299)

* examples: core: adds 2D camera two player split screen (#3298)

* Reviewed examples for consistency

* Update rtext.c

* Some code restructuring for input functions, consistency review

* Remove unneeded #if (#3301)

Co-authored-by: MichaelFiber <[email protected]>

* Revert "Disable UBSAN in zig builds. (#3292)" (#3303)

This reverts commit a316f9e.

Issue #1891 was fixed again, so this is no longer needed.

* rtextures: Fix ImageDraw() source clipping when drawing beyond top left (#3306)

* REVIEWED: `TextToPascal()` issue when first char is uppercase

* Implement FLAG_WINDOW_RESIZABLE for web (#3305)

Fixes #3231

* Update BINDINGS.md (#3307)

Fix Kaylib binding. Reroute to a new repository.
Binding renamed.

* Update webassembly.yml

* Add claw-raylib to BINDINGS.md (#3310)

* Add SetWindowMaxSize for desktop and web (#3309)

* Add SetWindowMaxSize for desktop and web

* Remove SizeInt and respective adjustments

* Update rtextures.c

* Reviewed parameters for consistency

* Rename windowM* to screenM* (#3312)

* Update BINDINGS.md (#3317)

Update TurboRaylib bindings

* Update rmodels.c

* Update BINDINGS.md with vaiorabbit/raylib-bindings (#3318)

* fixed spelling mistake

* put back parenthesis

* reverted major allignment changes

* reverted parser output changes

* reverted one more indentation change

---------

Co-authored-by: Brian-E <[email protected]>
Co-authored-by: Ray <[email protected]>
Co-authored-by: ubkp <[email protected]>
Co-authored-by: ashn <[email protected]>
Co-authored-by: actondev (Christos) <[email protected]>
Co-authored-by: vitopigno <[email protected]>
Co-authored-by: Asdqwe <[email protected]>
Co-authored-by: Jeffery Myers <[email protected]>
Co-authored-by: Ethan Simpson <[email protected]>
Co-authored-by: Nickolas McDonald <[email protected]>
Co-authored-by: Branimir Ričko <[email protected]>
Co-authored-by: iacore <[email protected]>
Co-authored-by: Ethan Conneely <[email protected]>
Co-authored-by: Johannes Barthelmes <[email protected]>
Co-authored-by: bXi <[email protected]>
Co-authored-by: Ryan Roden-Corrent <[email protected]>
Co-authored-by: Ikko Eltociear Ashimine <[email protected]>
Co-authored-by: SuperUserNameMan <[email protected]>
Co-authored-by: MichaelFiber <[email protected]>
Co-authored-by: MichaelFiber <[email protected]>
Co-authored-by: Dan Vu <[email protected]>
Co-authored-by: Gabriel dos Santos Sanches <[email protected]>
Co-authored-by: Rob Loach <[email protected]>
Co-authored-by: Peter0x44 <[email protected]>
Co-authored-by: Kenta <[email protected]>
Co-authored-by: bohonghuang <[email protected]>
Co-authored-by: turborium <[email protected]>
Co-authored-by: Wilson Silva <[email protected]>
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

No branches or pull requests

3 participants