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

[R-package] warning on R-devel with clang 15: 'format string is not a string literal' #6212

Closed
jameslamb opened this issue Nov 25, 2023 · 2 comments · Fixed by #6216
Closed

Comments

@jameslamb
Copy link
Collaborator

Description

With latest R-devel (R Under development (unstable) (2023-11-24 r85626)) + clang 15.0.7, R CMD check raises the following WARNING.

* checking whether package 'lightgbm' can be installed ... [108s/108s] WARNING
Found the following significant warnings:
  lightgbm_R.cpp:159:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:191:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  ....
full logs (click me)
Found the following significant warnings:
  lightgbm_R.cpp:159:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:191:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:216:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:243:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:260:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:308:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:320:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:330:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:362:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:400:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:419:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:430:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:440:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:451:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:462:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:478:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:493:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:507:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:538:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:548:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:558:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:568:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:579:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:590:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:599:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:608:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:629:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:637:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:648:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:658:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:668:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:717:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:732:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:744:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:756:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:794:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:813:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:848:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:876:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:905:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:934:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:951:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:979:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:1049:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:1073:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:1098:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:1109:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:1122:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:1146:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:1171:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:1191:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  lightgbm_R.cpp:1212:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
See '/__w/LightGBM/LightGBM/lightgbm.Rcheck/00install.out' for details.

Reproducible example

See the latest r-package (debian, R-devel, clang) CI job run on master (build link).

That can be reproduced using this configuration:

test-r-debian-clang:
name: r-package (debian, R-devel, clang)
timeout-minutes: 60
runs-on: ubuntu-latest
container: rhub/debian-clang-devel
steps:

Environment info

LightGBM version or commit hash: 2ee3ec8

Additional Comments

This would likely lead to a rejection of a new submission on CRAN.

It will also cause CI failures in this repo until it's either fixed or until we temporarily allow that WARNING in CI.

All of the lines noted in warnings are uses of the R_API_END() preprocessor macro

which looks like this:

#define R_API_END() } \
catch(LGBM_R_ErrorClass &cont) { R_ContinueUnwind(cont.cont_token); } \
catch(std::exception& ex) { LGBM_R_save_exception_msg(ex); } \
catch(std::string& ex) { LGBM_R_save_exception_msg(ex); } \
catch(...) { Rf_error("unknown exception"); } \
Rf_error(R_errmsg_buffer); \
return R_NilValue; /* <- won't be reached */

@jameslamb
Copy link
Collaborator Author

This is now being reported as a WARNING on the project's CRAN page as well.

image

https://cran.r-project.org/web/checks/check_results_lightgbm.html

I will try to put up a PR this week. @shiyu1994 if you get any emails from CRAN about this, please post them here.

@jameslamb
Copy link
Collaborator Author

Running R CMD INSTALL instead of R CMD CHECK, I saw more traceback information:

lightgbm_R.cpp:1191:3: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  R_API_END();
  ^~~~~~~~~~~
lightgbm_R.cpp:43:12: note: expanded from macro 'R_API_END'
  Rf_error(R_errmsg_buffer); \
           ^~~~~~~~~~~~~~~
lightgbm_R.cpp:1191:3: note: treat the string as an argument to avoid this
lightgbm_R.cpp:43:12: note: expanded from macro 'R_API_END'
  Rf_error(R_errmsg_buffer); \

That helped to narrow down where the issue was. It appears to be what's discussed over in llvm/llvm-project#56583.

As described in llvm/llvm-project#56583 (comment) specifically, something like printf("%d%d%d") could cause an overflow and the resulting stability and security issues that come with that.

clang is raising a warning here because there's no guarantee that Rf_error() won't be passed some string with formatting stuff like %s in it.

char R_errmsg_buffer[MAX_LENGTH_ERR_MSG];

Rf_error(R_errmsg_buffer); \

I put up #6216 with changes that'll hopefully fix that. I'll move it out of draft if CI passes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant