bji / libs3

Other
154 stars 151 forks source link

src/request.c:298: out of scope ? #88

Open dcb314 opened 5 years ago

dcb314 commented 5 years ago

[src/request.c:293] -> [src/request.c:290] -> [src/request.c:298]: (error) Using pointer to local variable 'headerNameWithPrefix' that is out of scope.

Source code is

if (addPrefix) {
    char headerNameWithPrefix[S3_MAX_METADATA_SIZE - sizeof(": v")];
    snprintf(headerNameWithPrefix, sizeof(headerNameWithPrefix),
             S3_METADATA_HEADER_NAME_PREFIX "%s", headerName);
    headerStr = headerNameWithPrefix;
}

// Make sure the new header (plus ": " plus string terminator) will fit
// in the buffer.
if ((values->amzHeadersRawLength + strlen(headerStr) + strlen(headerValue)
    + 3) >= sizeof(values->amzHeadersRaw)) {
aghiles commented 5 years ago

Looks like it indeed.

bji commented 5 years ago

I guess I missed that when I reviewed commit da1f0fb5e0f a couple of years ago.

Luckily, there are no other locals to push on the stack in that function; the only other local is a loop variable likely kept in a register by the compiler.

So I don't think this is likely to actually be a problem in any realistically compiled binaries, but it definitely should be fixed.

I am assuming that your compiler or whatever tool you are using to detect this error didn't find any other such problems in the code, which is at least encouraging.

dcb314 commented 5 years ago

I am assuming that your compiler or whatever tool you are using to detect this error didn't find any other such problems in the code, which is at least encouraging.

False assumption. I only gave you the most serious. Here is a warning it made:

src/request.c:844]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.

dcb314 commented 5 years ago

Some warnings from the compiler:

src/request.c:993:77: warning: '%s' directive output may be truncated writing up to 64 bytes into a region of size between 17 and 144 [-Wformat-truncation=] src/request.c:1059:74: warning: '%s' directive output may be truncated writing up to 2511 bytes into a region of size between 875 and 966 [-Wformat-truncation=] src/request.c:1461:51: warning: '%s' directive output may be truncated writing up to 64 bytes into a region of size between 31 and 96 [-Wformat-truncation=] src/request.c:1760:14: warning: '%s' directive output may be truncated writing up to 2511 bytes into a region of size between 170 and 329 [-Wformat-truncation=]

bji commented 5 years ago

OK I believe I fixed all in my recent commit 4f2db1ebfa82cc4b364a891e1deeaa73dfda9c24.

Except this; I'm not sure my compiler showed me this one:

src/request.c:844]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.

dcb314 commented 5 years ago

gcc flag -Wformat-signedness will help with the last one.

I used static analyser cppcheck, available from sourceforge. I recommend it for all C/C++ code.