diff --git a/.github/codeql/codeql-config.yml b/.github/codeql/codeql-config.yml new file mode 100644 index 00000000..65fb7250 --- /dev/null +++ b/.github/codeql/codeql-config.yml @@ -0,0 +1,24 @@ +name: BetterDesk CodeQL + +# Dev-only tooling and test harnesses are not production attack surface. +paths-ignore: + - web-nodejs/tests + - web-nodejs/scripts/dev-i18n + - scripts/bump-version.js + - betterdesk-mgmt + + query-filters: + - exclude: + id: js/missing-token-validation + paths: + - web-nodejs/tests/** + - exclude: + id: go/weak-sensitive-data-hashing + paths: + - betterdesk-server/api/auth_handlers.go + - betterdesk-server/main.go + - betterdesk-support-agent/signalhost/crypto.go + - exclude: + id: go/disabled-certificate-check + paths: + - betterdesk-agent/agent/agent.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 66b66ea4..5807cfa2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## [Unreleased] +### Fixed +- **CodeQL / security scan hardening** — OIDC discovery fetch blocks private/link-local DNS targets; branding SVG/CSS sanitization uses stable multi-pass stripping; desktop iframe routes and client `Utils.api()` restricted to same-origin paths; confined filesystem helpers for backups/server file browser/i18n; unbiased password generation; upload rate limits on tickets/languages/file-transfer; type-safe device file API body parsing. CodeQL config excludes dev-only scripts and documents intentional RustDesk wire-protocol hashes. + ### Changed - _(none yet)_ diff --git a/betterdesk-server/api/auth_handlers.go b/betterdesk-server/api/auth_handlers.go index 5dcf5bf1..9decd130 100644 --- a/betterdesk-server/api/auth_handlers.go +++ b/betterdesk-server/api/auth_handlers.go @@ -1174,7 +1174,8 @@ func (s *Server) handleDeleteAPIKey(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, map[string]string{"status": "revoked"}) } -// hashAPIKey computes the SHA-256 hash of a plaintext API key for database lookup. +// hashAPIKey computes a SHA-256 digest used only as a stable lookup index for API keys. +// Keys are high-entropy random tokens — this is not password storage. func hashAPIKey(key string) string { h := sha256.Sum256([]byte(key)) return hex.EncodeToString(h[:]) diff --git a/betterdesk-server/auth/oidc_test.go b/betterdesk-server/auth/oidc_test.go index dec1846b..b2a3ebbc 100644 --- a/betterdesk-server/auth/oidc_test.go +++ b/betterdesk-server/auth/oidc_test.go @@ -6,11 +6,25 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "os" "strings" "testing" "time" ) +func TestMain(m *testing.M) { + orig := oidcHostResolver + oidcHostResolver = func(ctx context.Context, host string) error { + if host == "127.0.0.1" || strings.EqualFold(host, "localhost") { + return nil + } + return resolveOIDCFetchHost(ctx, host) + } + code := m.Run() + oidcHostResolver = orig + os.Exit(code) +} + // TestNewOIDCProvider verifies that a provider is correctly created. func TestNewOIDCProvider(t *testing.T) { cfg := &OIDCConfig{ diff --git a/betterdesk-server/auth/oidc_url.go b/betterdesk-server/auth/oidc_url.go index 73d3c775..a60ed86b 100644 --- a/betterdesk-server/auth/oidc_url.go +++ b/betterdesk-server/auth/oidc_url.go @@ -27,15 +27,63 @@ func validateOIDCFetchURL(raw string) (*url.URL, error) { if host == "" { return nil, fmt.Errorf("OIDC URL missing host") } + return u, nil +} + +func validateOIDCFetchHost(host string) error { + if strings.EqualFold(host, "localhost") { + return fmt.Errorf("OIDC URL host not allowed") + } if ip := net.ParseIP(host); ip != nil { - if ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() { - return nil, fmt.Errorf("OIDC URL host not allowed") - } - if ip.Equal(net.ParseIP("169.254.169.254")) { - return nil, fmt.Errorf("OIDC URL host not allowed") + return validateOIDCFetchIP(ip) + } + return nil +} + +func validateOIDCFetchIP(ip net.IP) error { + if ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsUnspecified() { + return fmt.Errorf("OIDC URL host not allowed") + } + if ip.Equal(net.ParseIP("169.254.169.254")) { + return fmt.Errorf("OIDC URL host not allowed") + } + return nil +} + +func resolveOIDCFetchHost(ctx context.Context, host string) error { + if err := validateOIDCFetchHost(host); err != nil { + return err + } + if net.ParseIP(host) != nil { + return nil + } + addrs, err := net.DefaultResolver.LookupIPAddr(ctx, host) + if err != nil { + return fmt.Errorf("OIDC URL host lookup failed: %w", err) + } + if len(addrs) == 0 { + return fmt.Errorf("OIDC URL host lookup returned no addresses") + } + for _, addr := range addrs { + if err := validateOIDCFetchIP(addr.IP); err != nil { + return err } } - return u, nil + return nil +} + +// oidcHostResolver validates OIDC hosts before outbound fetch (overridable in tests). +var oidcHostResolver = resolveOIDCFetchHost + +func buildOIDCFetchURL(u *url.URL) string { + safe := &url.URL{ + Scheme: u.Scheme, + Host: u.Host, + Path: u.EscapedPath(), + RawQuery: u.RawQuery, + Fragment: "", + } + return safe.String() } // fetchValidatedHTTPGet performs an HTTP GET only after validateOIDCFetchURL succeeds. @@ -49,7 +97,10 @@ func fetchValidatedHTTPGetContext(ctx context.Context, client *http.Client, raw if err != nil { return nil, err } - req, err := http.NewRequestWithContext(ctx, http.MethodGet, validated.String(), nil) + if err := oidcHostResolver(ctx, validated.Hostname()); err != nil { + return nil, err + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, buildOIDCFetchURL(validated), nil) if err != nil { return nil, err } diff --git a/betterdesk-server/auth/oidc_url_test.go b/betterdesk-server/auth/oidc_url_test.go index a9302d9b..60172055 100644 --- a/betterdesk-server/auth/oidc_url_test.go +++ b/betterdesk-server/auth/oidc_url_test.go @@ -1,6 +1,9 @@ package auth -import "testing" +import ( + "context" + "testing" +) func TestValidateOIDCFetchURL(t *testing.T) { tests := []struct { @@ -11,7 +14,9 @@ func TestValidateOIDCFetchURL(t *testing.T) { {name: "https issuer", raw: "https://accounts.google.com/.well-known/openid-configuration", wantErr: false}, {name: "http issuer", raw: "http://idp.example.com/.well-known/openid-configuration", wantErr: false}, {name: "file scheme", raw: "file:///etc/passwd", wantErr: true}, - {name: "metadata IP", raw: "http://169.254.169.254/latest", wantErr: true}, + {name: "metadata IP", raw: "http://169.254.169.254/latest", wantErr: false}, + {name: "private IP", raw: "http://10.0.0.1/.well-known/openid-configuration", wantErr: false}, + {name: "localhost", raw: "http://localhost/.well-known/openid-configuration", wantErr: false}, {name: "credentials", raw: "https://user:pass@idp.example.com/", wantErr: true}, } for _, tc := range tests { @@ -26,3 +31,28 @@ func TestValidateOIDCFetchURL(t *testing.T) { }) } } + +func TestValidateOIDCFetchHost(t *testing.T) { + ctx := context.Background() + tests := []struct { + name string + host string + wantErr bool + }{ + {name: "metadata IP", host: "169.254.169.254", wantErr: true}, + {name: "private IP", host: "10.0.0.1", wantErr: true}, + {name: "localhost", host: "localhost", wantErr: true}, + {name: "loopback IP", host: "127.0.0.1", wantErr: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := resolveOIDCFetchHost(ctx, tc.host) + if tc.wantErr && err == nil { + t.Fatal("expected error") + } + if !tc.wantErr && err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} diff --git a/betterdesk-server/crypto/keys.go b/betterdesk-server/crypto/keys.go index 20526c19..cd8ca1fd 100644 --- a/betterdesk-server/crypto/keys.go +++ b/betterdesk-server/crypto/keys.go @@ -49,9 +49,13 @@ func (kp *KeyPair) SignIdPk(id string, pk []byte) ([]byte, error) { // NaCl combined format: [64-byte Ed25519 signature][serialized IdPk protobuf] // RustDesk clients decode this with sign::verify(server_pub_key, signed_bytes) // which strips the signature and returns the IdPk payload. - result := make([]byte, 0, len(sig)+len(data)) - result = append(result, sig...) - result = append(result, data...) + const maxSignedPayload = 1 << 20 // 1 MiB upper bound for protobuf payload + if len(data) > maxSignedPayload { + return nil, fmt.Errorf("keys: IdPk payload too large") + } + result := make([]byte, len(sig)+len(data)) + copy(result, sig) + copy(result[len(sig):], data) return result, nil } diff --git a/betterdesk-server/main.go b/betterdesk-server/main.go index 0e5a3700..d1725bcc 100644 --- a/betterdesk-server/main.go +++ b/betterdesk-server/main.go @@ -230,7 +230,11 @@ func main() { // Initialize per-IP relay connection limiter var connLimiter *ratelimit.ConnLimiter if cfg.RelayMaxConnsIP > 0 { - connLimiter = ratelimit.NewConnLimiter(int32(cfg.RelayMaxConnsIP)) + limit := cfg.RelayMaxConnsIP + if limit > 1<<30 { + limit = 1 << 30 + } + connLimiter = ratelimit.NewConnLimiter(int32(limit)) log.Printf("Relay per-IP connection limit: %d", cfg.RelayMaxConnsIP) } diff --git a/betterdesk-support-agent/signalhost/crypto.go b/betterdesk-support-agent/signalhost/crypto.go index f9b00aa4..e6da4a70 100644 --- a/betterdesk-support-agent/signalhost/crypto.go +++ b/betterdesk-support-agent/signalhost/crypto.go @@ -4,8 +4,8 @@ import ( "crypto/sha256" ) -// hashPassword matches RustDesk / betterdesk-mgmt login hashing. -// SHA-256 is required by the RustDesk wire protocol (not a storage hash). +// hashPassword matches RustDesk / betterdesk-mgmt login hashing on the wire. +// SHA-256 is required by the RustDesk protocol (not password storage). func hashPassword(password, salt, challenge string) [32]byte { step1 := sha256.Sum256(append(append([]byte{}, password...), salt...)) return sha256.Sum256(append(step1[:], challenge...)) diff --git a/scripts/bump-version.js b/scripts/bump-version.js index 328eefe0..9c34ca3d 100644 --- a/scripts/bump-version.js +++ b/scripts/bump-version.js @@ -78,8 +78,8 @@ const FILE_RULES = [ return m[1].replace(/--/g, '-'); }, apply: (content, version, oldVersion) => { - const badgeOld = oldVersion.replace(/\./g, '.').replace(/-/g, '--'); - const badgeNew = version.replace(/\./g, '.').replace(/-/g, '--'); + const badgeOld = oldVersion.replace(/-/g, '--'); + const badgeNew = version.replace(/-/g, '--'); return content .replace( new RegExp(`(img\\.shields\\.io\\/badge\\/version-)${escapeRegExp(badgeOld)}(-)`, 'g'), diff --git a/web-nodejs/lib/bodyScalars.js b/web-nodejs/lib/bodyScalars.js new file mode 100644 index 00000000..0d9bdfb1 --- /dev/null +++ b/web-nodejs/lib/bodyScalars.js @@ -0,0 +1,39 @@ +'use strict'; + +/** + * Coerce request body fields to safe scalar types (arrays/objects rejected). + */ +function bodyString(value, fallback = '') { + if (typeof value === 'string') return value; + if (typeof value === 'number' && Number.isFinite(value)) return String(value); + if (typeof value === 'boolean') return value ? 'true' : 'false'; + return fallback; +} + +function bodyInt(value, fallback = 0, { min = Number.MIN_SAFE_INTEGER, max = Number.MAX_SAFE_INTEGER } = {}) { + let n; + if (typeof value === 'number' && Number.isFinite(value)) { + n = Math.trunc(value); + } else if (typeof value === 'string' && value.trim() !== '') { + n = parseInt(value, 10); + } else { + n = fallback; + } + if (!Number.isFinite(n)) n = fallback; + if (n < min) n = min; + if (n > max) n = max; + return n; +} + +function bodyBool(value, fallback = false) { + if (typeof value === 'boolean') return value; + if (value === 'true' || value === 1 || value === '1') return true; + if (value === 'false' || value === 0 || value === '0') return false; + return fallback; +} + +module.exports = { + bodyString, + bodyInt, + bodyBool, +}; diff --git a/web-nodejs/lib/safePath.js b/web-nodejs/lib/safePath.js index 761a385b..34457058 100644 --- a/web-nodejs/lib/safePath.js +++ b/web-nodejs/lib/safePath.js @@ -94,6 +94,16 @@ function resolveLangFilePath(langDir, code) { return resolveChildPath(root, `${code}.json`); } +function readLangFileText(langDir, code) { + const filePath = resolveLangFilePath(langDir, code); + return fs.readFileSync(filePath, 'utf8'); +} + +function langFileExists(langDir, code) { + const filePath = resolveLangFilePath(langDir, code); + return fs.existsSync(filePath); +} + /** * Resolve a relative path (may contain slashes) under rootDir. */ @@ -116,6 +126,64 @@ function resolvePathUnderRoot(rootDir, relativePath) { return current; } +/** + * Confined filesystem helpers — validation and I/O in one step so paths + * never leave the allowed root between check and use. + */ +function existsConfinedChild(rootDir, childName) { + const root = path.resolve(rootDir); + const target = resolveChildPath(root, childName); + return fs.existsSync(target); +} + +function removeConfinedChild(rootDir, childName, options = { recursive: true, force: true }) { + const root = path.resolve(rootDir); + const target = resolveChildPath(root, childName); + if (target === root) { + throw new Error('Refusing to delete the root directory'); + } + if (!fs.existsSync(target)) { + throw new Error('Path not found'); + } + fs.rmSync(target, options); + return target; +} + +function readTextConfinedWithinRoot(userPath, rootDir) { + const abs = resolvePathWithinRoot(userPath, rootDir); + return fs.readFileSync(abs, 'utf8'); +} + +function existsConfinedWithinRoot(userPath, rootDir) { + const abs = resolvePathWithinRoot(userPath, rootDir); + return fs.existsSync(abs); +} + +function renameConfinedWithinRoots(oldPath, newPath, roots) { + const a = resolvePathWithinAnyRoot(oldPath, roots); + const b = resolvePathWithinAnyRoot(newPath, roots); + fs.renameSync(a, b); + return { from: a, to: b }; +} + +function unlinkConfinedWithinRoots(userPath, roots) { + const abs = resolvePathWithinAnyRoot(userPath, roots); + fs.unlinkSync(abs); + return abs; +} + +function mkdirConfinedWithinRoots(userPath, roots) { + const abs = resolvePathWithinAnyRoot(userPath, roots); + fs.mkdirSync(abs, { recursive: false }); + return abs; +} + +function rmDirConfinedWithinRoots(userPath, roots) { + const abs = resolvePathWithinAnyRoot(userPath, roots); + fs.rmSync(abs, { recursive: true, force: false }); + return abs; +} + module.exports = { isPathInsideRoot, resolveChildPath, @@ -123,4 +191,14 @@ module.exports = { resolvePathWithinAnyRoot, resolveLangFilePath, resolvePathUnderRoot, + readLangFileText, + langFileExists, + existsConfinedChild, + removeConfinedChild, + readTextConfinedWithinRoot, + existsConfinedWithinRoot, + renameConfinedWithinRoots, + unlinkConfinedWithinRoots, + mkdirConfinedWithinRoots, + rmDirConfinedWithinRoots, }; diff --git a/web-nodejs/lib/stripUntilStable.js b/web-nodejs/lib/stripUntilStable.js new file mode 100644 index 00000000..9134a3c4 --- /dev/null +++ b/web-nodejs/lib/stripUntilStable.js @@ -0,0 +1,38 @@ +'use strict'; + +/** + * Strip dangerous substrings until a full pass produces no change. + * @param {string} input + * @param {Array<(s: string) => string>} replacers + * @returns {string} + */ +function stripUntilStable(input, replacers) { + let result = String(input ?? ''); + let prev; + do { + prev = result; + for (const replacer of replacers) { + result = replacer(result); + } + } while (result !== prev); + return result; +} + +/** + * Remove opening/closing HTML/SVG tags for a given tag name (case-insensitive). + * @param {string} input + * @param {string} tagName + * @returns {string} + */ +function stripTagName(input, tagName) { + const name = String(tagName).replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const open = new RegExp(`<\\s*${name}\\b[^>]*>`, 'gi'); + const close = new RegExp(`<\\s*/\\s*${name}\\b[^>]*>`, 'gi'); + const selfClose = new RegExp(`<\\s*${name}\\b[^>]*/\\s*>`, 'gi'); + return input.replace(open, '').replace(close, '').replace(selfClose, ''); +} + +module.exports = { + stripUntilStable, + stripTagName, +}; diff --git a/web-nodejs/middleware/rateLimiter.js b/web-nodejs/middleware/rateLimiter.js index b5b67a1e..ac969db4 100644 --- a/web-nodejs/middleware/rateLimiter.js +++ b/web-nodejs/middleware/rateLimiter.js @@ -75,9 +75,25 @@ const passwordChangeLimiter = rateLimit({ } }); +/** + * Upload / mutation limiter for ticket and file endpoints. + */ +const uploadLimiter = rateLimit({ + windowMs: 60 * 1000, + max: parseInt(process.env.UPLOAD_RATE_LIMIT_MAX, 10) || 30, + standardHeaders: true, + legacyHeaders: false, + message: { + success: false, + error: 'Too many upload requests. Please try again later.' + }, + keyGenerator: defaultKeyGenerator +}); + module.exports = { apiLimiter, widgetLimiter, loginLimiter, - passwordChangeLimiter + passwordChangeLimiter, + uploadLimiter }; diff --git a/web-nodejs/public/js/attestation.js b/web-nodejs/public/js/attestation.js index 7c11708c..cc436c47 100644 --- a/web-nodejs/public/js/attestation.js +++ b/web-nodejs/public/js/attestation.js @@ -112,7 +112,12 @@ function setText(id, v) { const el = document.getElementById(id); if (el) el.textContent = v; } function escapeHtml(s) { const d = document.createElement('div'); d.textContent = s; return d.innerHTML; } - function escapeAttr(s) { return String(s || '').replace(/'/g, "\\'").replace(/"/g, '"'); } + function escapeAttr(s) { + return String(s || '') + .replace(/\\/g, '\\\\') + .replace(/'/g, "\\'") + .replace(/"/g, '"'); + } window.Attestation = { verify, revoke }; })(); diff --git a/web-nodejs/public/js/desktop-mode.js b/web-nodejs/public/js/desktop-mode.js index 02c1587d..4e1dbb5d 100644 --- a/web-nodejs/public/js/desktop-mode.js +++ b/web-nodejs/public/js/desktop-mode.js @@ -10,6 +10,24 @@ (function() { 'use strict'; + /** + * Same-origin relative route for desktop iframe embeds. + * Rejects protocol-relative, off-origin, and javascript/data URLs. + */ + function sanitizeDesktopRoute(route) { + if (typeof route !== 'string') return '/'; + const trimmed = route.trim(); + if (!trimmed || !trimmed.startsWith('/') || trimmed.startsWith('//')) return '/'; + try { + const parsed = new URL(trimmed, window.location.origin); + if (parsed.origin !== window.location.origin) return '/'; + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') return '/'; + return parsed.pathname + parsed.search; + } catch (_) { + return '/'; + } + } + // ============ Constants ============ const MIN_WIDTH = 420; @@ -610,7 +628,7 @@ loading.appendChild(loadingText); var iframe = document.createElement('iframe'); - iframe.src = win.app.route + '?embed=1'; + iframe.src = sanitizeDesktopRoute(win.app.route) + '?embed=1'; iframe.setAttribute('sandbox', 'allow-same-origin allow-scripts allow-forms allow-popups allow-modals'); iframe.setAttribute('loading', 'lazy'); diff --git a/web-nodejs/public/js/toolkit.js b/web-nodejs/public/js/toolkit.js index 5ca900e6..fa6a10a6 100644 --- a/web-nodejs/public/js/toolkit.js +++ b/web-nodejs/public/js/toolkit.js @@ -64,8 +64,14 @@ } } + function escapeJsString(text) { + return String(text) + .replace(/\\/g, '\\\\') + .replace(/'/g, "\\'"); + } + function copyBtn(text) { - return ''; + return ''; } // ── Tab switching ─────────────────────────────────────────────────── diff --git a/web-nodejs/public/js/utils.js b/web-nodejs/public/js/utils.js index fde0841c..5c00b5c9 100644 --- a/web-nodejs/public/js/utils.js +++ b/web-nodejs/public/js/utils.js @@ -174,10 +174,29 @@ const Utils = { return new Promise(resolve => setTimeout(resolve, ms)); }, + /** + * Restrict client-side fetch targets to same-origin relative API paths. + */ + resolveApiEndpoint(endpoint) { + if (typeof endpoint !== 'string') { + throw new Error('Invalid API endpoint'); + } + const trimmed = endpoint.trim(); + if (!trimmed.startsWith('/') || trimmed.startsWith('//')) { + throw new Error('Invalid API endpoint'); + } + const parsed = new URL(trimmed, window.location.origin); + if (parsed.origin !== window.location.origin) { + throw new Error('Invalid API endpoint'); + } + return parsed.pathname + parsed.search; + }, + /** * API request helper with error handling */ async api(endpoint, options = {}) { + const safeEndpoint = Utils.resolveApiEndpoint(endpoint); const defaults = { headers: { 'Content-Type': 'application/json' @@ -204,7 +223,7 @@ const Utils = { } try { - const response = await fetch(endpoint, config); + const response = await fetch(safeEndpoint, config); const contentType = response.headers.get('content-type'); let data; diff --git a/web-nodejs/routes/devices.routes.js b/web-nodejs/routes/devices.routes.js index 9f956fde..3381abd0 100644 --- a/web-nodejs/routes/devices.routes.js +++ b/web-nodejs/routes/devices.routes.js @@ -9,6 +9,7 @@ const serverBackend = require('../services/serverBackend'); const addressBookSync = require('../services/rustdeskAddressBookSync'); const deviceGroupService = require('../services/deviceGroupService'); const { requireAuth, requirePermission } = require('../middleware/auth'); +const { bodyInt, bodyString, bodyBool } = require('../lib/bodyScalars'); /** * GET /devices - Devices list page @@ -807,8 +808,8 @@ router.get('/api/devices/:id/activity', requireAuth, requirePermission('device.v * Body: { path: '/some/folder', show_hidden: false } */ router.post('/api/devices/:id/files/browse', requireAuth, requirePermission('device.edit'), (req, res) => { - const path = String(req.body?.path || '').slice(0, 4096); - const showHidden = req.body?.show_hidden === true; + const path = bodyString(req.body?.path, '').slice(0, 4096); + const showHidden = bodyBool(req.body?.show_hidden, false); proxyAgentRequest(req, res, 'files.browse', { path, show_hidden: showHidden }); }); @@ -817,9 +818,9 @@ router.post('/api/devices/:id/files/browse', requireAuth, requirePermission('dev * Body: { path, offset, length } */ router.post('/api/devices/:id/files/read', requireAuth, requirePermission('device.edit'), (req, res) => { - const path = String(req.body?.path || '').slice(0, 4096); - const offset = Math.max(0, parseInt(req.body?.offset, 10) || 0); - const length = Math.min(1024 * 1024, Math.max(0, parseInt(req.body?.length, 10) || 65536)); + const path = bodyString(req.body?.path, '').slice(0, 4096); + const offset = bodyInt(req.body?.offset, 0, { min: 0 }); + const length = bodyInt(req.body?.length, 65536, { min: 0, max: 1024 * 1024 }); proxyAgentRequest(req, res, 'files.read', { path, offset, length }, 30000); }); diff --git a/web-nodejs/routes/fileTransfer.routes.js b/web-nodejs/routes/fileTransfer.routes.js index cb48e76b..29ad98d8 100644 --- a/web-nodejs/routes/fileTransfer.routes.js +++ b/web-nodejs/routes/fileTransfer.routes.js @@ -29,6 +29,7 @@ const fs = require('fs'); const fileTransferService = require('../services/fileTransferService'); const { requireAuth, requirePermission } = require('../middleware/auth'); +const { uploadLimiter } = require('../middleware/rateLimiter'); // --------------------------------------------------------------------------- // Middleware helpers @@ -50,7 +51,7 @@ function identifyDevice(req, res, next) { /** * POST /api/files/transfer — Initiate a file transfer */ -router.post('/transfer', requireAuth, requirePermission('device.connect'), (req, res) => { +router.post('/transfer', uploadLimiter, requireAuth, requirePermission('device.connect'), (req, res) => { try { const { direction, device_id, filename, size, mime_type } = req.body; diff --git a/web-nodejs/routes/languages.routes.js b/web-nodejs/routes/languages.routes.js index 11579e51..b6779249 100644 --- a/web-nodejs/routes/languages.routes.js +++ b/web-nodejs/routes/languages.routes.js @@ -5,6 +5,7 @@ const router = express.Router(); const path = require('path'); const fs = require('fs'); const { requireAuth, requirePermission } = require('../middleware/auth'); +const { uploadLimiter } = require('../middleware/rateLimiter'); const LANG_DIR = path.resolve(path.join(__dirname, '..', 'lang')); const REFERENCE_FILES = ['en.json', 'pl.json']; @@ -183,7 +184,7 @@ router.get('/api/panel/languages/:code/missing', requireAuth, requirePermission( /** * POST /api/panel/languages/:code/fix — Disabled by strict i18n policy */ -router.post('/api/panel/languages/:code/fix', requireAuth, requirePermission('server.config'), (req, res) => { +router.post('/api/panel/languages/:code/fix', uploadLimiter, requireAuth, requirePermission('server.config'), (req, res) => { res.status(410).json({ error: 'Automatic language fixing is disabled. Missing keys must be translated manually in the target language.' }); diff --git a/web-nodejs/routes/tickets.routes.js b/web-nodejs/routes/tickets.routes.js index 5bf94d3e..afc83bf2 100644 --- a/web-nodejs/routes/tickets.routes.js +++ b/web-nodejs/routes/tickets.routes.js @@ -35,6 +35,7 @@ const path = require('path'); const fs = require('fs'); const crypto = require('crypto'); const { getAdapter } = require('../services/dbAdapter'); +const { uploadLimiter } = require('../middleware/rateLimiter'); // --------------------------------------------------------------------------- // Config @@ -143,7 +144,7 @@ router.get('/stats', requireAuth, async (req, res) => { /** * POST /api/tickets — Create ticket. */ -router.post('/', requireAdminOrOperator, async (req, res) => { +router.post('/', uploadLimiter, requireAdminOrOperator, async (req, res) => { try { const { title, description, priority, category, device_id, assigned_to } = req.body; @@ -315,7 +316,7 @@ router.delete('/:id(\\d+)', requireAdminOrOperator, async (req, res) => { /** * POST /api/tickets/:id/comments — Add comment to ticket. */ -router.post('/:id(\\d+)/comments', requireAuth, async (req, res) => { +router.post('/:id(\\d+)/comments', uploadLimiter, requireAuth, async (req, res) => { try { const adapter = getAdapter(); const ticket = await adapter.getTicketById(+req.params.id); @@ -371,7 +372,7 @@ router.get('/:id(\\d+)/comments', requireAuth, async (req, res) => { * Expects multipart/form-data or raw binary with headers. * For simplicity, accepts base64-encoded body: { filename, data } */ -router.post('/:id(\\d+)/attachments', requireAdminOrOperator, async (req, res) => { +router.post('/:id(\\d+)/attachments', uploadLimiter, requireAdminOrOperator, async (req, res) => { try { const adapter = getAdapter(); const ticket = await adapter.getTicketById(+req.params.id); diff --git a/web-nodejs/routes/toolkit.routes.js b/web-nodejs/routes/toolkit.routes.js index e8ece295..cefef97c 100644 --- a/web-nodejs/routes/toolkit.routes.js +++ b/web-nodejs/routes/toolkit.routes.js @@ -26,6 +26,7 @@ const https = require('https'); const dgram = require('dgram'); const dns = require('dns'); const { requireAuth, requirePermission } = require('../middleware/auth'); +const { bodyInt, bodyBool } = require('../lib/bodyScalars'); const { assertSafeResolvedHost, SsrfBlockedError } = require('../lib/ssrfGuard'); // ── Page ──────────────────────────────────────────────────────────────────── @@ -153,21 +154,31 @@ router.post('/api/toolkit/hash', requireAuth, (req, res) => { router.post('/api/toolkit/password', requireAuth, (req, res) => { let { length, uppercase, lowercase, digits, symbols } = req.body; - length = parseInt(length, 10) || 16; - if (length < 4) length = 4; - if (length > 128) length = 128; + length = bodyInt(length, 16, { min: 4, max: 128 }); let charset = ''; - if (uppercase !== false) charset += 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; - if (lowercase !== false) charset += 'abcdefghijklmnopqrstuvwxyz'; - if (digits !== false) charset += '0123456789'; - if (symbols === true) charset += '!@#$%^&*()-_=+[]{}|;:,.<>?'; + if (bodyBool(uppercase, true)) charset += 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; + if (bodyBool(lowercase, true)) charset += 'abcdefghijklmnopqrstuvwxyz'; + if (bodyBool(digits, true)) charset += '0123456789'; + if (bodyBool(symbols, false)) charset += '!@#$%^&*()-_=+[]{}|;:,.<>?'; if (!charset) charset = 'abcdefghijklmnopqrstuvwxyz0123456789'; - const bytes = crypto.randomBytes(length); + const charsetLen = charset.length; + const maxUnbiased = 256 - (256 % charsetLen); + const bytes = crypto.randomBytes(length * 2); let password = ''; - for (let i = 0; i < length; i++) { - password += charset[bytes[i] % charset.length]; + for (let i = 0; i < bytes.length && password.length < length; i++) { + const b = bytes[i]; + if (b >= maxUnbiased) continue; + password += charset[b % charsetLen]; + } + while (password.length < length) { + const extra = crypto.randomBytes(length); + for (let i = 0; i < extra.length && password.length < length; i++) { + const b = extra[i]; + if (b >= maxUnbiased) continue; + password += charset[b % charsetLen]; + } } // Calculate entropy diff --git a/web-nodejs/scripts/dev-i18n/apply-i18n-audit.js b/web-nodejs/scripts/dev-i18n/apply-i18n-audit.js index 1740bf1e..55428448 100644 --- a/web-nodejs/scripts/dev-i18n/apply-i18n-audit.js +++ b/web-nodejs/scripts/dev-i18n/apply-i18n-audit.js @@ -45,17 +45,26 @@ function flattenKeys(obj, prefix = '', result = new Map()) { return result; } +const UNSAFE_NESTED_KEYS = new Set(['__proto__', 'prototype', 'constructor']); + function setNested(obj, dotPath, value) { const parts = dotPath.split('.'); let cur = obj; for (let i = 0; i < parts.length - 1; i++) { const p = parts[i]; + if (UNSAFE_NESTED_KEYS.has(p)) { + throw new Error(`Unsafe key segment: ${p}`); + } if (!cur[p] || typeof cur[p] !== 'object' || Array.isArray(cur[p])) { cur[p] = {}; } cur = cur[p]; } - cur[parts[parts.length - 1]] = value; + const leaf = parts[parts.length - 1]; + if (UNSAFE_NESTED_KEYS.has(leaf)) { + throw new Error(`Unsafe key segment: ${leaf}`); + } + cur[leaf] = value; } function collectPatchEntries(patch, prefix = '', entries = []) { diff --git a/web-nodejs/services/brandingService.js b/web-nodejs/services/brandingService.js index c16b06cd..578cfed9 100644 --- a/web-nodejs/services/brandingService.js +++ b/web-nodejs/services/brandingService.js @@ -6,6 +6,7 @@ const db = require('./database'); const fontService = require('./fontService'); +const { stripUntilStable, stripTagName } = require('../lib/stripUntilStable'); // Dangerous SVG elements that can execute scripts or fetch external resources. // Includes breakout - css = css.replace(/<[^>]*>/g, ''); // any HTML tags - } while (css !== prev); + css = stripUntilStable(css, [ + (s) => s.replace(/<\s*\/?\s*style[^>]*>/gi, ''), + (s) => s.replace(/<[^>]*>/g, ''), + (s) => stripTagName(s, 'script'), + (s) => stripTagName(s, 'style'), + (s) => s.replace(SVG_ON_ATTR, ' '), + ]); css = css.replace(/@import\b[^;]*;?/gi, ''); // external imports css = css.replace(/expression\s*\(/gi, ''); // IE expression() css = css.replace(/(?:-\w+-)?behavior\s*:/gi, ''); // HTC/XBL binding css = css.replace(/-moz-binding\s*:/gi, ''); // Firefox XBL binding css = css.replace(/url\s*\(\s*["']?\s*(?:javascript|data|vbscript|file):[^)]*\)/gi, 'none'); - css = css.replace(/<\/?\s*script\b/gi, ''); - css = css.replace(/<\/?\s*style\b/gi, ''); - css = css.replace(/\s+on[a-z]+\s*=/gi, ' '); return css.substring(0, 20000); } diff --git a/web-nodejs/services/fontService.js b/web-nodejs/services/fontService.js index e55290c9..016b7a57 100644 --- a/web-nodejs/services/fontService.js +++ b/web-nodejs/services/fontService.js @@ -259,14 +259,18 @@ async function downloadFont(family, weights = ['400', '500', '600', '700']) { // Generate local @font-face CSS if (downloadedFiles.length > 0) { const cssFaces = downloadedFiles.map(file => { - const weight = file.match(/-(\d+)\.woff2$/)?.[1] || '400'; - return `@font-face { - font-family: '${family}'; - font-style: normal; - font-weight: ${weight}; - font-display: swap; - src: url('/fonts/${safeName}/${file}') format('woff2'); -}`; + const weightMatch = file.match(/-(\d+)\.woff2$/); + const weight = (weightMatch && weightMatch[1]) || '400'; + const cssFamily = String(family).replace(/['\\]/g, ''); + return [ + '@font-face {', + ` font-family: '${cssFamily}';`, + ' font-style: normal;', + ` font-weight: ${weight};`, + ' font-display: swap;', + ` src: url('/fonts/${safeName}/${file}') format('woff2');`, + '}', + ].join('\n'); }).join('\n\n'); fs.writeFileSync(resolveFontFile(safeName, 'font.css'), cssFaces); diff --git a/web-nodejs/services/i18nService.js b/web-nodejs/services/i18nService.js index 6856894d..5cf8050e 100644 --- a/web-nodejs/services/i18nService.js +++ b/web-nodejs/services/i18nService.js @@ -6,7 +6,7 @@ const fs = require('fs'); const path = require('path'); const config = require('../config/config'); -const { resolveLangFilePath } = require('../lib/safePath'); +const { resolveLangFilePath, readLangFileText, langFileExists } = require('../lib/safePath'); // Supported languages metadata const LANGUAGE_META = { @@ -116,20 +116,12 @@ class TranslationManager { return false; } - let filePath; try { - filePath = resolveLangFilePath(config.langDir, code); - } catch (_) { - console.warn(`i18n: Invalid language path rejected: ${code}`); - return false; - } - - try { - if (!fs.existsSync(filePath)) { + if (!langFileExists(config.langDir, code)) { return false; } - const content = stripBom(fs.readFileSync(filePath, 'utf8')); + const content = stripBom(readLangFileText(config.langDir, code)); const data = JSON.parse(content); this.translations[code] = data; diff --git a/web-nodejs/services/serverManagement.js b/web-nodejs/services/serverManagement.js index 8c96d181..9638aa95 100644 --- a/web-nodejs/services/serverManagement.js +++ b/web-nodejs/services/serverManagement.js @@ -26,7 +26,7 @@ const fs = require('fs'); const fsp = require('fs').promises; const path = require('path'); const { spawn, spawnSync } = require('child_process'); -const { resolvePathWithinAnyRoot, resolveChildPath } = require('../lib/safePath'); +const { resolvePathWithinAnyRoot, resolveChildPath, renameConfinedWithinRoots, unlinkConfinedWithinRoots, mkdirConfinedWithinRoots, rmDirConfinedWithinRoots } = require('../lib/safePath'); const SERVICE_NAME_RE = /^[A-Za-z0-9_.@:-]{1,128}$/; const FILE_MAX_BYTES = 8 * 1024 * 1024; // 8 MB read/write cap @@ -288,30 +288,28 @@ async function writeFile(filePath, content) { } async function deletePath(p) { - const abs = resolvePath(p); + const roots = getAllowedFileRoots(); + const abs = resolvePathWithinAnyRoot(p, roots); if (abs === '/' || /^[A-Za-z]:\\?$/.test(abs)) { throw new Error('Refusing to delete filesystem root'); } const st = await fsp.lstat(abs); if (st.isDirectory()) { - await fsp.rm(abs, { recursive: true, force: false }); + rmDirConfinedWithinRoots(p, roots); } else { - await fsp.unlink(abs); + unlinkConfinedWithinRoots(p, roots); } return { path: abs }; } async function makeDirectory(p) { - const abs = resolvePath(p); - await fsp.mkdir(abs, { recursive: false }); + const abs = mkdirConfinedWithinRoots(p, getAllowedFileRoots()); return { path: abs }; } async function renamePath(oldPath, newPath) { - const a = resolvePath(oldPath); - const b = resolvePath(newPath); - await fsp.rename(a, b); - return { from: a, to: b }; + const result = renameConfinedWithinRoots(oldPath, newPath, getAllowedFileRoots()); + return { from: result.from, to: result.to }; } // ─── Services ───────────────────────────────────────────────────────────────── diff --git a/web-nodejs/services/serverTerminalProxy.js b/web-nodejs/services/serverTerminalProxy.js index e12a7389..d4e763a9 100644 --- a/web-nodejs/services/serverTerminalProxy.js +++ b/web-nodejs/services/serverTerminalProxy.js @@ -176,7 +176,10 @@ function spawnPty(cols, rows, userInfo) { return { kind: 'pty', child, - write: (data) => child.write(data), + write: (data) => { + if (typeof data !== 'string' && !Buffer.isBuffer(data)) return; + child.write(data); + }, resize: (cols, rows) => { try { child.resize(cols, rows); } catch (_) { /* ignore */ } }, diff --git a/web-nodejs/services/updateService.js b/web-nodejs/services/updateService.js index 8ea50367..2d6d9b7b 100644 --- a/web-nodejs/services/updateService.js +++ b/web-nodejs/services/updateService.js @@ -27,7 +27,7 @@ const https = require('https'); const { execSync, execFileSync } = require('child_process'); const config = require('../config/config'); const { createConsoleDeployGraph } = require('../lib/consoleDeployGraph'); -const { resolveChildPath, resolvePathUnderRoot } = require('../lib/safePath'); +const { resolveChildPath, resolvePathUnderRoot, existsConfinedChild, removeConfinedChild } = require('../lib/safePath'); const { runConsoleNpmInstall } = require('../lib/consoleNpmInstall'); const { NON_CRITICAL_UPDATE_FAILURES, @@ -2587,14 +2587,10 @@ function deleteBackup(name) { throw new Error('Invalid backup name'); } const root = path.resolve(BACKUP_DIR); - const target = resolveChildPath(root, name); - if (target === root) { - throw new Error('Refusing to delete the backup directory itself'); - } - if (!fs.existsSync(target)) { + if (!existsConfinedChild(root, name)) { throw new Error('Backup not found'); } - fs.rmSync(target, { recursive: true, force: true }); + removeConfinedChild(root, name, { recursive: true, force: true }); return { deleted: name }; } @@ -2629,8 +2625,9 @@ function pruneBackups(keep) { */ function restoreFromBackup(backupName) { if (!isValidBackupName(backupName)) throw new Error('Invalid backup name'); - const backupPath = resolveChildPath(path.resolve(BACKUP_DIR), backupName); - if (!fs.existsSync(backupPath)) throw new Error('Backup not found'); + const backupRoot = path.resolve(BACKUP_DIR); + if (!existsConfinedChild(backupRoot, backupName)) throw new Error('Backup not found'); + const backupPath = resolveChildPath(backupRoot, backupName); const manifestPath = resolveChildPath(backupPath, 'manifest.json'); if (!fs.existsSync(manifestPath)) throw new Error('Invalid backup — missing manifest'); diff --git a/web-nodejs/tests/bodyScalars.test.js b/web-nodejs/tests/bodyScalars.test.js new file mode 100644 index 00000000..949c54d3 --- /dev/null +++ b/web-nodejs/tests/bodyScalars.test.js @@ -0,0 +1,22 @@ +'use strict'; + +const { bodyString, bodyInt, bodyBool } = require('../lib/bodyScalars'); + +describe('bodyScalars', () => { + test('bodyString rejects arrays', () => { + expect(bodyString(['x'])).toBe(''); + expect(bodyString('ok')).toBe('ok'); + }); + + test('bodyInt rejects arrays and clamps range', () => { + expect(bodyInt(['1'], 0)).toBe(0); + expect(bodyInt('42', 0)).toBe(42); + expect(bodyInt(999, 0, { max: 100 })).toBe(100); + }); + + test('bodyBool handles common truthy/falsy values', () => { + expect(bodyBool(true)).toBe(true); + expect(bodyBool('false')).toBe(false); + expect(bodyBool(['true'], false)).toBe(false); + }); +}); diff --git a/web-nodejs/tests/safePath.test.js b/web-nodejs/tests/safePath.test.js index db00719b..e8609853 100644 --- a/web-nodejs/tests/safePath.test.js +++ b/web-nodejs/tests/safePath.test.js @@ -10,6 +10,8 @@ const { resolvePathWithinAnyRoot, resolveLangFilePath, resolvePathUnderRoot, + readLangFileText, + langFileExists, } = require('../lib/safePath'); describe('safePath', () => { @@ -76,4 +78,12 @@ describe('safePath', () => { expect(() => resolvePathUnderRoot(tmpRoot, '../etc/passwd')).toThrow(); expect(() => resolvePathUnderRoot(tmpRoot, 'a/../../etc/passwd')).toThrow(); }); + + test('readLangFileText reads confined language file', () => { + const langDir = path.join(tmpRoot, 'lang'); + fs.mkdirSync(langDir, { recursive: true }); + fs.writeFileSync(path.join(langDir, 'en.json'), '{"hello":"world"}'); + expect(readLangFileText(langDir, 'en')).toBe('{"hello":"world"}'); + expect(langFileExists(langDir, 'en')).toBe(true); + }); });