Skip to content

GCC 10.3: Wformat-overflow#17

Open
ax3l wants to merge 7 commits into
AMReX-Codes:mainfrom
ax3l:fix-Wformat-overflow
Open

GCC 10.3: Wformat-overflow#17
ax3l wants to merge 7 commits into
AMReX-Codes:mainfrom
ax3l:fix-Wformat-overflow

Conversation

@ax3l

@ax3l ax3l commented Mar 16, 2022

Copy link
Copy Markdown
Member

Spotted buffer overflows in sprintf with GCC 10.3.

  • fix me

X-ref: ignores AMReX-Codes/amrex#2750 for now (can be a separate PR)

@etpalmer63

Copy link
Copy Markdown
Contributor

Some quick grepping leads to

Dataset.cpp:  char maxInfoV[Amrvis::LINELENGTH],  minInfoV[Amrvis::LINELENGTH];

Which leads to:

./amrex/Src/Extern/amrdata/AMReX_AmrvisConstants.H:const int LINELENGTH = 160;

I guess we cannot just change LINELENGTH because it affects how AMRDATA behaves. (see amrex/Extern/amrdata/AMReX_AmrData.cpp. I will ask wiser people.

@ax3l

ax3l commented Mar 17, 2022

Copy link
Copy Markdown
Member Author

Jup, it's basically writing a string added to a prefix in a same-size string a couple of time.

I would say we trash the C pointer logic and make it C++? :)

@WeiqunZhang

Copy link
Copy Markdown
Member

Yes, there are quite some C strings and sprintf's that should be replaced with C++ strings.

ax3l added 2 commits March 17, 2022 10:37
Spotted buffer overflows in `sprintf` with GCC 10.3
@ax3l ax3l force-pushed the fix-Wformat-overflow branch from 6f677d5 to c3c74b4 Compare March 17, 2022 17:41
Fix string buffer overflows in two files.
@ax3l

ax3l commented Mar 17, 2022

Copy link
Copy Markdown
Member Author

@WeiqunZhang can you please take a look at AMReX-Codes/amrex#2660 ? :)
That one I do not immediately get, the other things I fixed now in Amrvis.

Comment thread Dataset.cpp Outdated
Comment thread Dataset.cpp Outdated
Comment thread Dataset.cpp
sprintf(minInfoV, fstring, rMin);
sprintf(minInfo, "Min:%s", minInfoV);
XmString sNewMin = XmStringCreateSimple(minInfo);
std::string minInfo("Min:");

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not change this for compatibility, but we could add a space here:

Suggested change
std::string minInfo("Min:");
std::string minInfo("Min: ");

Comment thread Dataset.cpp
sprintf(maxInfoV, fstring, rMax);
sprintf(maxInfo, "Max:%s", maxInfoV);
XmString sNewMax = XmStringCreateSimple(maxInfo);
std::string maxInfo("Max:");

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not change this for compatibility, but we could add a space here:

Suggested change
std::string maxInfo("Max:");
std::string maxInfo("Max: ");

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.

3 participants