Actions
Bug #5539
closedlandlock: coverity warnings
Affected Versions:
Effort:
Difficulty:
Label:
Description
** CID 1514671: Error handling issues (CHECKED_RETURN) /src/util-landlock.c: 181 in LandlockSandboxing() ________________________________________________________________________________________________________ *** CID 1514671: Error handling issues (CHECKED_RETURN) /src/util-landlock.c: 181 in LandlockSandboxing() 175 } 176 177 void LandlockSandboxing(SCInstance *suri) 178 { 179 /* Read configuration variable and exit if no enforcement */ 180 int conf_status; >>> CID 1514671: Error handling issues (CHECKED_RETURN) >>> Calling "ConfGetBool" without checking return value (as is done elsewhere 30 out of 31 times). 181 ConfGetBool("security.landlock.enabled", &conf_status); 182 if (!conf_status) { 183 SCLogConfig("Landlock is not enabled in configuration"); 184 return; 185 } 186 struct landlock_ruleset *ruleset = LandlockCreateRuleset(); ** CID 1514670: Security best practices violations (TOCTOU) /src/util-landlock.c: 204 in LandlockSandboxing() ________________________________________________________________________________________________________ *** CID 1514670: Security best practices violations (TOCTOU) /src/util-landlock.c: 204 in LandlockSandboxing() 198 if (suri->run_mode == RUNMODE_PCAP_FILE) { 199 const char *pcap_file; 200 ConfGet("pcap-file.file", &pcap_file); 201 char *file_name = SCStrdup(pcap_file); 202 if (file_name != NULL) { 203 struct stat statbuf; >>> CID 1514670: Security best practices violations (TOCTOU) >>> Calling function "stat" to perform check on "file_name". 204 if (stat(file_name, &statbuf) != -1) { 205 if (S_ISDIR(statbuf.st_mode)) { 206 LandlockSandboxingReadPath(ruleset, file_name); 207 } else { 208 LandlockSandboxingReadPath(ruleset, dirname(file_name)); 209 } ** CID 1514669: (CHECKED_RETURN) /src/util-landlock.c: 248 in LandlockSandboxing() /src/util-landlock.c: 200 in LandlockSandboxing() ________________________________________________________________________________________________________ *** CID 1514669: (CHECKED_RETURN) /src/util-landlock.c: 248 in LandlockSandboxing() 242 } else { 243 LandlockSandboxingWritePath(ruleset, LOCAL_STATE_DIR "/run/suricata/"); 244 } 245 } 246 if (suri->sig_file_exclusive == FALSE) { 247 const char *rule_path; >>> CID 1514669: (CHECKED_RETURN) >>> Calling "ConfGet" without checking return value (as is done elsewhere 87 out of 89 times). 248 ConfGet("default-rule-path", &rule_path); 249 if (rule_path) { 250 LandlockSandboxingReadPath(ruleset, rule_path); 251 } 252 } 253 /src/util-landlock.c: 200 in LandlockSandboxing() 194 if (stat(ConfigGetDataDirectory(), &sb) == 0) { 195 LandlockSandboxingAddRule(ruleset, ConfigGetDataDirectory(), 196 _LANDLOCK_SURI_ACCESS_FS_WRITE | _LANDLOCK_ACCESS_FS_READ); 197 } 198 if (suri->run_mode == RUNMODE_PCAP_FILE) { 199 const char *pcap_file; >>> CID 1514669: (CHECKED_RETURN) >>> Calling "ConfGet" without checking return value (as is done elsewhere 87 out of 89 times). 200 ConfGet("pcap-file.file", &pcap_file); 201 char *file_name = SCStrdup(pcap_file); 202 if (file_name != NULL) { 203 struct stat statbuf; 204 if (stat(file_name, &statbuf) != -1) { 205 if (S_ISDIR(statbuf.st_mode)) {
The retval checking is pretty trivial. Not sure how the TOCTOU would be solved in this case. @Philippe Antoine any thoughts?
Updated by Philippe Antoine about 2 years ago
No satisfying answer.
open the file and use fstat rather than stat may silence coverity...
Updated by Victor Julien about 2 years ago
- Target version changed from 7.0.0-beta1 to 7.0.0-rc1
Updated by Victor Julien almost 2 years ago
- Target version changed from 7.0.0-rc1 to 7.0.0-rc2
Updated by Philippe Antoine almost 2 years ago
ping @Eric Leblond What are the news on this ?
Updated by Eric Leblond almost 2 years ago
I've pushed https://github.com/regit/suricata/tree/landlock-coverity using the fstat solution but I don't know if coverity will go for a test. The fix is not really a fix.
Updated by Philippe Antoine almost 2 years ago
- Status changed from Assigned to In Review
Updated by Philippe Antoine almost 2 years ago
I think the TOCTOU is indeed not really a problem as LandlockSandboxingReadPath
does good checks later
Updated by Victor Julien over 1 year ago
- Target version changed from 7.0.0-rc2 to 7.0.0
Updated by Victor Julien over 1 year ago
- Target version changed from 7.0.0 to 7.0.1
Updated by Victor Julien about 1 year ago
- Target version changed from 7.0.1 to 7.0.2
Updated by Victor Julien about 1 year ago
- Target version changed from 7.0.2 to 7.0.3
Updated by Victor Julien 12 months ago
- Target version changed from 7.0.3 to 8.0.0-beta1
- Label Needs backport to 7.0 added
Updated by Philippe Antoine 3 months ago
- Assignee changed from Eric Leblond to Philippe Antoine
Taking on this
Updated by Philippe Antoine 3 months ago
- Related to Bug #5704: Filestore is not working if landlock is enabled added
Updated by Philippe Antoine 3 months ago
- Status changed from In Review to In Progress
Updated by Philippe Antoine 3 months ago
- Status changed from In Progress to Resolved
Was fixed by 9cb0bc3332300a902c78d0e86dfcd0d5115f6803
Updated by Philippe Antoine 3 months ago
- Status changed from Resolved to Closed
Was solved before 8 branch was started
Actions