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

TDSLoader: remove readString maxLength #22651

Merged
merged 3 commits into from
Oct 8, 2021
Merged

TDSLoader: remove readString maxLength #22651

merged 3 commits into from
Oct 8, 2021

Conversation

tomsoftware
Copy link
Contributor

  • removed readString maxLength that is not defined by 3ds spec
  • clean up code

Description

***removed maxLength ***

The using of a max-length when reading strings from a Chunk is not defined in the spec:

Format is "ASCIIZ" - no max 64
http://paulbourke.net/dataformats/3ds/
or
http://www.martinreddy.net/gfx/3d/3DS.spec

first item of Subchunk 4000 is an ASCIIZ string of the objects name.
ASCIIZ means a string of charakters ended by a zero.

This leads to problems parsing files with e.g. NameObjects having name-length > 64

I think this issue was intudused by the source implementation using for that code, that is a poor C implementation using fixed size char buffers to store the names
https://github.com/hoopoe/lib3ds/blob/f8cfd93e7a4f81feeb8473f0b73b0397e9ebf996/src/lib3ds_file.c#L226
and needs to prevent buffer-overflow

Other implementations do not use this max-length

*** clean up ***
the original code uses a "global" read-position indicator and functions changing this - that is quite complicated

* removed readString maxLength that is not defined by 3ds spec
* clean up code
@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 7, 2021

/ping @tentone

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 7, 2021

fix syntax error
fix let -> const
@tomsoftware
Copy link
Contributor Author

sorry... for the missing "{" in the first commit.
-> is there a test set of .3ds files?

@tentone
Copy link
Contributor

tentone commented Oct 8, 2021

Change looks good! Thanks!

I can confirm that lib3ds was used as a reference for the initial version.

@mrdoob mrdoob added this to the r134 milestone Oct 8, 2021
@mrdoob mrdoob merged commit 14250fa into mrdoob:dev Oct 8, 2021
@mrdoob
Copy link
Owner

mrdoob commented Oct 8, 2021

Thanks!

@tomsoftware tomsoftware deleted the TDSLoader-fix-readString branch October 10, 2021 14:25
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.

4 participants