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

StrIntUtils.cpp:204: array index used before sanity check ? #1441

Closed
dcb314 opened this issue Mar 19, 2015 · 5 comments
Closed

StrIntUtils.cpp:204: array index used before sanity check ? #1441

dcb314 opened this issue Mar 19, 2015 · 5 comments
Assignees
Milestone

Comments

@dcb314
Copy link

@dcb314 dcb314 commented Mar 19, 2015

StrIntUtils.cpp:204]: (style) Array index 'i' is used before limits check.

while (data[i] == ' ' && i < str.size()) {

Looking at the bigger picture, it seems to me that the entire stringToUnsignedNumeric
could be replaced by a call into the standard C or C++ library.

@FooBarWidget

This comment has been minimized.

Copy link
Member

@FooBarWidget FooBarWidget commented Mar 19, 2015

You're right. The size check should come before the content check. Thanks for spotting this.

stringTo(Un)signedNumeric cannot be replaced by atoi or a similar standard library call, because atoi and family requires NULL-terminated strings. stringTo(Un)signedNumeric is designed to work with strings that have no termination character.

@FooBarWidget FooBarWidget self-assigned this Mar 19, 2015
FooBarWidget added a commit that referenced this issue Mar 19, 2015
Thanks to dcb314 for spotting this. Closes GH-1441.
@dcb314

This comment has been minimized.

Copy link
Author

@dcb314 dcb314 commented Mar 19, 2015

stringTo(Un)signedNumeric cannot be replaced by atoi or a similar standard library call, because atoi >and family requires NULL-terminated strings.

Sounds like nonsense to me. Your local variable "data" is a NULL terminated string.

Are you new to C programming ? It looks to me like you are making a newbie error.

@FooBarWidget

This comment has been minimized.

Copy link
Member

@FooBarWidget FooBarWidget commented Mar 19, 2015

dcb314, I appreciate your contribution, but I would prefer it if you refrain from such insinuations.

'data' is a char *, but it is not a NULL-terminated string. It is a pointer to an array of chars which may or may not be not terminated by a nul. For example, the input may be a substring of another string, or it may refer to a network data buffer which has no termination character.

@FooBarWidget

This comment has been minimized.

Copy link
Member

@FooBarWidget FooBarWidget commented Mar 19, 2015

Note that the input 'str' is a StaticString, which is our own lightweight wrapper around a 'char *' with a size field. If 'str' is an std::string and if I called 'str.c_str()' instead of 'str.data()', then you would have been right.

@tinco

This comment has been minimized.

Copy link
Contributor

@tinco tinco commented Mar 19, 2015

Hi @dcb314, I noticed that you've reported a lot of issues to C projects on github, how did you come across Passenger? Are you a user? Did you find the bug reading through the source or did you use a static analysis tool?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.