raylib security audit results, by Radically Open Security (ROS)

[raysan5]

Recently raylib went through a security audit by Radically Open Security and I decided to make public the final report with the main security concerns found. I already reviewed all the concerns and took some actions. Here the details.

Full report: raylib_security_audit_report_by_ROS.pdf

Security concerns reviewed with actions taken

• CLN-002 — Command Injection in OpenURL()

  ACTION: Added additional URL validation checks to minimize attacks f8e42cb

  REMARK: A pending future action will be replacing system() call by the new Process API: [rcore] Process manipulation API for Windows and POSIX-compliant platforms #5822

• CLN-005 — Buffer Overflow in LoadAutomationEventList()

  ACTION: Added limits to data size read from string, to limit to target string size (64 bytes). ab35e0f

  REMARK: Automation system should be redesigned and probably disabled by default with SUPPORT_AUTOMATION_EVENTS=0

• CLN-006 — Heap Out-of-Bounds Write in LoadAutomationEventList()

  ACTION: Added capacity check to avoid out-of bounds ab35e0f

  REMARK: Automation system should be redesigned and probably disabled by default with SUPPORT_AUTOMATION_EVENTS=0

• CLN-007 — Out-of-Bounds Read/Write in ImageFromImage()

  ACTION: Added rec validation to avoid any memory out-of-bounds read/write c31666f

  NOTE: raylib by design expect valid input data provided by users, to keep code simpler. Still, in the last few years multiple data validation checks have been introduced in more critical points. Also note that usual raylib strategy is fallback to some valid path and log warnings with detected issues.

• CLN-012 — Use of Assertions in tinyobj_loader_c.h

  ACTION: Replaced assert() by runtime checks 97d2b01

  NOTE: This issue has been detected on an external library (tinyobj_loader_c.h) and should be addressed on base repo, usage can also be disabled with SUPPORT_FILEFORMAT_OBJ=0. Actually any professional application should avoid usage intermediate file-formats and process+package only rquired data of those formats into a packed format, for example the free and open-specs rres file-format.

• CLN-013 — Integer Overflow

  ACTION: Reviewed GetPixelDataSize() to check for data requirements >2GB, limit to 2GB pixel data sizes. Log warning message. cfe56d9

  REMARK: Reviewed additional functions: rlGetPixelDataSize(), GenImageColor(), ImageRotate()

• CLN-015 — TraceLog() Interprets Caller-controlled Text as a Format String

  ACTION: Mitigation, reviewed message construction, replaced strncpy() usage by memcpy(). Not a solution for the problem but users with a more strict control of logging should use a custom logging function with the provided callback: SetTraceLogCallback() 5992929

• CLN-016 — Data Loaders Accept Path Traversal Inside Asset Metadata

  ACTION: None (for now). Dependant on external intermediate file formats: .fnt (font), .obj/.mtl/.iqm (3d models)

  NOTE: It is not reecommended to use those intermediate file-formats as final file formats in professional applications. Data should be extracted, validated and properly packed into a binary file format like rres.

• CLN-018 — Text Helpers Expose Distinct Out-of-bounds Behaviors

  ACTION: Added additional checks to avoid out-of-bounds memory accesses in affected functions: TextJoin(), TextSubtext(), TextInsert*() ebec978

Feel free to comment! 💯

[nikita-edel]

Does anyone happen to know which static analysis tools they used?

[raysan5]

You can read the report, it's listed there.

[nikita-edel]

Ah ty, didnt see the document