Project

General

Profile

Actions

Bug #19457

closed

Enforce stricter restriction on authorized node id and hostname

Added by Alexis Mousset almost 3 years ago. Updated 8 months ago.

Status:
Released
Priority:
N/A
Category:
Security
Target version:
Severity:
Minor - inconvenience | misleading | easy workaround
UX impact:
User visibility:
Operational - other Techniques | Rudder settings | Plugins
Effort required:
Very Small
Priority:
0
Name check:
To do
Fix check:
Checked
Regression:

Description

Currently the webapp allows anything in the [a-zA-Z0-9\-] range (which includes things like --insecure while on agent side the inventory check script is much stricter and checks for:

($uuid ne "root" \&\& $uuid !~ /[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/i))

As this check is already present at inventory creation, we can apply it pretty safely on the webapp side, or a at least prevent dash as first char.

This would avoid option injection in commands using node id as argument.

For hostnames, currently toto " $ <b>tutu</b><script>alert(1);</script> is accepted as a valid hostname.

Given the hostname is used in several places, including command arguments, it could be a good thing to restrict its content to a reasonable char set to prevent various injections.


Related issues 2 (0 open2 closed)

Related to Rudder - Bug #19456: Lack of HTML escaping in nodes listReleasedNicolas CHARLESActions
Has duplicate Rudder - Bug #19458: Validate the hostname fieldRejectedActions
Actions #1

Updated by Alexis Mousset almost 3 years ago

  • Description updated (diff)
Actions #2

Updated by François ARMAND almost 3 years ago

  • Related to Bug #19456: Lack of HTML escaping in nodes list added
Actions #3

Updated by François ARMAND almost 3 years ago

  • Subject changed from Enforce stricter restriction on authorized node id to Enforce stricter restriction on authorized node id and hostname

I'm merging #19458 with that one, since the check will happen at the same place (when an inventory is processed for UX, and when an inventory is saved for security and deal with case where the inventory is not processed in the default path).

Actions #4

Updated by François ARMAND almost 3 years ago

  • Description updated (diff)
  • Severity set to Minor - inconvenience | misleading | easy workaround
  • User visibility set to Operational - other Techniques | Rudder settings | Plugins
  • Effort required set to Very Small
  • Priority changed from 0 to 61
Actions #5

Updated by François ARMAND almost 3 years ago

  • Status changed from New to In progress
  • Assignee set to François ARMAND
Actions #6

Updated by François ARMAND almost 3 years ago

  • Has duplicate Bug #19458: Validate the hostname field added
Actions #7

Updated by François ARMAND almost 3 years ago

  • Status changed from In progress to Pending technical review
  • Assignee changed from François ARMAND to Vincent MEMBRÉ
  • Pull Request set to https://github.com/Normation/rudder/pull/3686
Actions #8

Updated by François ARMAND almost 3 years ago

  • Status changed from Pending technical review to Pending release
Actions #9

Updated by Alexis Mousset over 2 years ago

  • Fix check changed from To do to Checked
Actions #10

Updated by Vincent MEMBRÉ over 2 years ago

  • Status changed from Pending release to Released

This bug has been fixed in Rudder 6.1.14 and 6.2.8 which were released today.

Actions #11

Updated by Alexis Mousset 8 months ago

  • Private changed from Yes to No
  • Priority changed from 61 to 0
Actions

Also available in: Atom PDF