owasp-modsecurity / owasp-modsecurity/ModSecurity

SecUploadFileMode parsed as decimal instead of octal; temp files have no read permission

Open Beginner friendly
#3,580 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

3.x
Dominant language
C++
Stars
9.8k
Forks
1.8k
Avg merge
2h 46m
Merged PRs (30d)
1

Description

Description

Multipart upload temporary files (the ones referenced by FILES_TMPNAMES) are created with incorrect permissions. With the default SecUploadFileMode 0600, the resulting temp file has mode 1130 (---x-wx--T) instead of 0600 (-rw-------). All read bits are stripped, so @inspectFile scripts, and any other consumer of the temp file, cannot read the uploaded file.

Root Cause

The octal string 0600 is being parsed as the decimal number 600, and 600(decimal) == 1130(octal). The value is then passed straight to fchmod().

This is a regression: it works on v3.0.14 and is broken on v3.0.15.

Logs & Dumps

[!NOTE]
Debug logs don't help here, as they simply show everything working fine.

To Reproduce

Steps to reproduce the behavior:

  1. modsecurity.conf:
    SecRequestBodyAccess On
    SecUploadDir /tmp/
    SecTmpDir /tmp/
    SecUploadKeepFiles On
    SecUploadFileMode 0600
    SecRule FILES_TMPNAMES "@inspectFile /path/to/script.sh" \
        "id:1234,phase:2,deny"
    
  2. Upload a file:
    curl -F "file=@sample.txt" http://localhost/
    
  3. Inspect the temp file in SecUploadDir:
    ls -l /tmp/*-file-*
    
    -> mode is 1130, not 0600.
  4. The @inspectFile script cannot read the file (no read bit), so inspection can either fail or behave unexpected.
Expected Behavior

The temp file is created with the configured octal mode (0600 -> -rw-------), as documented ("The default mode (0600) only grants read/write access to the account writing the file"), so @inspectFile and other consumers can read it.

Server (please complete the following information):

Rule Set (please complete the following information):

  • Running any public or commercial rule set? Not required to reproduce; a single custom @inspectFile rule is enough.
  • What is the version number? N/A

Additional Context

Confirmed against current v3/master, the mode is applied verbatim:
https://github.com/owasp-modsecurity/ModSecurity/blob/f5a6fcd37aea3a7618408b7d1f03b94de9164637/src/request_body_processor/multipart.cc#L89-L98

fchmod() treats mode as literal permission bits, so m_uploadFileMode.m_value must contain octal 0600 (384(decimal)). Instead it contains 600 decimal (or 01130(octal)), which is exactly "0600" parsed in base 10.

The mis-parse is in the directive handling, in src/parser/seclang-parser.yy:

So "0600" -> 600(decimal) -> fchmod(fd, 600) -> 01130(octal). Besides breaking readers of FILES_TMPNAMES, this silently produces unintended/insecure permission bits for any configured SecUploadFileMode value (here: sticky bit set, group gains write+execute, owner loses read/write).

Workaround

Any @inspectFile script can simply ensure the file at the path passed to it has read access. Despite the discrepancy, it nonetheless has the authority to do so.

Suggested Fix

Parse SecUploadFileMode as octal: mirror SecAuditLogFileMode's strtol(..., NULL, 8), or have the upload-file-mode config parse with base 8 (or base 0 to honor a leading "0").

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/parser/seclang-parser.yy around the SecUploadFileMode handling and compare it with the nearby SecAuditLogFileMode parsing. Reproduce the configuration with SecUploadFileMode 0600 and an upload, then verify that the temporary file mode is 0600 and readable by the @inspectFile consumer.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.