From a0eebfcb3fe8709375c93cbca1ed494f1ff60bce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Z=C3=B6ller?= Date: Tue, 7 Apr 2026 10:28:10 +0200 Subject: [PATCH] Security Fixes --- .../fitpub/controller/AuthController.java | 11 +- .../fitpub/controller/TimelineController.java | 16 ++- .../fitpub/repository/ActivityRepository.java | 5 + .../fitpub/security/JwtTokenProvider.java | 32 +++++- .../fitpub/service/TimelineService.java | 13 ++- .../fitpub/util/GpxFileValidator.java | 5 +- .../net/javahippie/fitpub/util/GpxParser.java | 5 +- .../fitpub/util/SecureXmlFactories.java | 48 ++++++++ src/main/resources/application.yml | 4 +- src/main/resources/static/css/fitpub.css | 11 ++ src/main/resources/static/js/timeline.js | 107 ++++++++++++++++-- .../templates/activities/detail.html | 12 +- .../resources/templates/activities/list.html | 35 +++++- .../resources/templates/timeline/public.html | 12 ++ 14 files changed, 279 insertions(+), 37 deletions(-) create mode 100644 src/main/java/net/javahippie/fitpub/util/SecureXmlFactories.java diff --git a/src/main/java/net/javahippie/fitpub/controller/AuthController.java b/src/main/java/net/javahippie/fitpub/controller/AuthController.java index f690d96..f83d984 100644 --- a/src/main/java/net/javahippie/fitpub/controller/AuthController.java +++ b/src/main/java/net/javahippie/fitpub/controller/AuthController.java @@ -51,17 +51,14 @@ public class AuthController { .body(null); } - // Check registration password if configured - // Check for both null and blank (empty or whitespace-only strings) - log.debug("Registration password check - configured: '{}', provided: '{}'", - configuredRegistrationPassword, request.getRegistrationPassword()); - + // Check registration password if configured. + // NEVER log the configured or provided password values — they are credentials. if (configuredRegistrationPassword != null && !configuredRegistrationPassword.trim().isEmpty()) { String providedPassword = request.getRegistrationPassword(); if (providedPassword == null || providedPassword.trim().isEmpty() || !configuredRegistrationPassword.equals(providedPassword)) { - log.warn("Registration attempt with invalid registration password for username: {} (expected: '{}', got: '{}')", - request.getUsername(), configuredRegistrationPassword, providedPassword); + log.warn("Registration attempt with invalid registration password for username: {}", + request.getUsername()); throw new IllegalArgumentException("Invalid registration password"); } log.info("Registration password validated successfully for username: {}", request.getUsername()); diff --git a/src/main/java/net/javahippie/fitpub/controller/TimelineController.java b/src/main/java/net/javahippie/fitpub/controller/TimelineController.java index 7598446..9cb0def 100644 --- a/src/main/java/net/javahippie/fitpub/controller/TimelineController.java +++ b/src/main/java/net/javahippie/fitpub/controller/TimelineController.java @@ -100,14 +100,20 @@ public class TimelineController { @AuthenticationPrincipal(errorOnInvalidType = false) UserDetails userDetails, @RequestParam(defaultValue = "0") int page, @RequestParam(defaultValue = "20") int size, - @RequestParam(required = false) String search + @RequestParam(required = false) String search, + @RequestParam(required = false) String hashtag ) { UUID userId = null; if (userDetails != null) { userId = getUserId(userDetails); - log.debug("Public timeline request from authenticated user: {} (search: {})", userId, search); + log.debug("Public timeline request from authenticated user: {} (search: {}, hashtag: {})", userId, search, hashtag); } else { - log.debug("Public timeline request (unauthenticated) (search: {})", search); + log.debug("Public timeline request (unauthenticated) (search: {}, hashtag: {})", search, hashtag); + } + + // Reject malformed hashtags (extraction only allows \w) + if (hashtag != null && !hashtag.matches("\\w+")) { + hashtag = null; } // Sort by activity start date descending (latest first) @@ -115,9 +121,9 @@ public class TimelineController { // Use search if filters provided, otherwise use standard timeline Page timeline; - if (search != null) { + if (search != null || hashtag != null) { timeline = timelineService.searchPublicTimeline( - userId, search, pageable + userId, search, hashtag, pageable ); } else { timeline = timelineService.getPublicTimeline(userId, pageable); diff --git a/src/main/java/net/javahippie/fitpub/repository/ActivityRepository.java b/src/main/java/net/javahippie/fitpub/repository/ActivityRepository.java index bf3ca87..35a181b 100644 --- a/src/main/java/net/javahippie/fitpub/repository/ActivityRepository.java +++ b/src/main/java/net/javahippie/fitpub/repository/ActivityRepository.java @@ -369,6 +369,10 @@ public interface ActivityRepository extends JpaRepository { OR LOWER(a.description) LIKE LOWER(CONCAT('%', :searchText, '%')) OR LOWER(a.activity_location) LIKE LOWER(CONCAT('%', :searchText, '%')) )) + AND (CAST(:hashtagPattern AS text) IS NULL OR ( + a.title ~* CAST(:hashtagPattern AS text) + OR a.description ~* CAST(:hashtagPattern AS text) + )) GROUP BY a.id, a.user_id, a.activity_type, a.title, a.description, a.started_at, a.ended_at, a.timezone, a.visibility, a.total_distance, a.total_duration_seconds, a.elevation_gain, a.elevation_loss, a.simplified_track, a.track_points_json, a.created_at, a.updated_at, @@ -377,6 +381,7 @@ public interface ActivityRepository extends JpaRepository { """, nativeQuery = true) Page searchPublicTimeline(@Param("visibility") String visibility, @Param("searchText") String searchText, + @Param("hashtagPattern") String hashtagPattern, @Param("currentUserId") UUID currentUserId, Pageable pageable); diff --git a/src/main/java/net/javahippie/fitpub/security/JwtTokenProvider.java b/src/main/java/net/javahippie/fitpub/security/JwtTokenProvider.java index a7f4416..219cafc 100644 --- a/src/main/java/net/javahippie/fitpub/security/JwtTokenProvider.java +++ b/src/main/java/net/javahippie/fitpub/security/JwtTokenProvider.java @@ -23,16 +23,44 @@ public class JwtTokenProvider { private final SecretKey secretKey; private final long validityInMilliseconds; + /** + * Known insecure placeholder values that may have leaked into deployment configs. + * The bean refuses to initialize if the configured secret matches any of them. + */ + private static final java.util.Set KNOWN_PLACEHOLDERS = java.util.Set.of( + "change-this-secret-key-in-production-must-be-at-least-32-characters-long", + "changeme" + ); + public JwtTokenProvider( - @Value("${fitpub.security.jwt.secret}") String secret, + @Value("${fitpub.security.jwt.secret:}") String secret, @Value("${fitpub.security.jwt.expiration:86400000}") long validityInMilliseconds ) { + if (secret == null || secret.isBlank()) { + throw new IllegalStateException( + "JWT secret is not configured. Set the JWT_SECRET environment variable to a random value of at least 32 characters." + ); + } + if (KNOWN_PLACEHOLDERS.contains(secret)) { + throw new IllegalStateException( + "JWT secret is set to a known placeholder value. Generate a real secret (e.g. `openssl rand -base64 48`) and set JWT_SECRET." + ); + } // Ensure secret is long enough for HS256 (at least 256 bits / 32 bytes) if (secret.getBytes(StandardCharsets.UTF_8).length < 32) { - throw new IllegalArgumentException("JWT secret must be at least 32 characters long"); + throw new IllegalStateException( + "JWT secret must be at least 32 bytes long (HS256 requirement). Current length: " + + secret.getBytes(StandardCharsets.UTF_8).length + ); } this.secretKey = Keys.hmacShaKeyFor(secret.getBytes(StandardCharsets.UTF_8)); this.validityInMilliseconds = validityInMilliseconds; + // Log a fingerprint, never the secret itself + log.info("JWT signing key initialised (length={} bytes, fingerprint={}…{})", + secret.getBytes(StandardCharsets.UTF_8).length, + secret.substring(0, Math.min(4, secret.length())), + secret.substring(Math.max(0, secret.length() - 4)) + ); } /** diff --git a/src/main/java/net/javahippie/fitpub/service/TimelineService.java b/src/main/java/net/javahippie/fitpub/service/TimelineService.java index 38bc6ad..b5234da 100644 --- a/src/main/java/net/javahippie/fitpub/service/TimelineService.java +++ b/src/main/java/net/javahippie/fitpub/service/TimelineService.java @@ -216,18 +216,27 @@ public class TimelineService { public Page searchPublicTimeline( UUID userId, String searchText, + String hashtag, Pageable pageable ) { - log.debug("Searching public timeline (userId: {}, search: {})", - userId, searchText); + log.debug("Searching public timeline (userId: {}, search: {}, hashtag: {})", + userId, searchText, hashtag); // Create unsorted Pageable since ORDER BY is already in the native query Pageable unsortedPageable = PageRequest.of(pageable.getPageNumber(), pageable.getPageSize()); + // Build a POSIX regex matching #hashtag as a standalone token (case-insensitive via ~*). + // The hashtag value contains only \w characters (extraction enforces this), so no escaping needed. + String hashtagPattern = null; + if (hashtag != null && !hashtag.isBlank()) { + hashtagPattern = "(^|[^[:alnum:]_])#" + hashtag + "([^[:alnum:]_]|$)"; + } + // Use optimized search query with JOINs and WHERE conditions Page results = activityRepository.searchPublicTimeline( Activity.Visibility.PUBLIC.name(), searchText, + hashtagPattern, userId, unsortedPageable ); diff --git a/src/main/java/net/javahippie/fitpub/util/GpxFileValidator.java b/src/main/java/net/javahippie/fitpub/util/GpxFileValidator.java index 5e8d8e5..95572c1 100644 --- a/src/main/java/net/javahippie/fitpub/util/GpxFileValidator.java +++ b/src/main/java/net/javahippie/fitpub/util/GpxFileValidator.java @@ -65,9 +65,8 @@ public class GpxFileValidator { */ private void validateGpxStructure(byte[] fileData) { try { - // Parse XML to check well-formedness - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setNamespaceAware(true); + // Parse XML to check well-formedness, using a hardened factory (defends against XXE). + DocumentBuilderFactory factory = SecureXmlFactories.newDocumentBuilderFactory(true); DocumentBuilder builder = factory.newDocumentBuilder(); Document doc = builder.parse(new ByteArrayInputStream(fileData)); diff --git a/src/main/java/net/javahippie/fitpub/util/GpxParser.java b/src/main/java/net/javahippie/fitpub/util/GpxParser.java index d5461c0..1bcb7e8 100644 --- a/src/main/java/net/javahippie/fitpub/util/GpxParser.java +++ b/src/main/java/net/javahippie/fitpub/util/GpxParser.java @@ -59,9 +59,8 @@ public class GpxParser { ParsedActivityData parsedData = new ParsedActivityData(); parsedData.setSourceFormat("GPX"); - // Parse XML - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setNamespaceAware(true); + // Parse XML using a hardened factory (defends against XXE). + DocumentBuilderFactory factory = SecureXmlFactories.newDocumentBuilderFactory(true); DocumentBuilder builder = factory.newDocumentBuilder(); Document doc = builder.parse(new ByteArrayInputStream(fileData)); diff --git a/src/main/java/net/javahippie/fitpub/util/SecureXmlFactories.java b/src/main/java/net/javahippie/fitpub/util/SecureXmlFactories.java new file mode 100644 index 0000000..cd34055 --- /dev/null +++ b/src/main/java/net/javahippie/fitpub/util/SecureXmlFactories.java @@ -0,0 +1,48 @@ +package net.javahippie.fitpub.util; + +import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; + +/** + * Shared factory helpers that produce XML parsers hardened against XXE + * (XML External Entity) attacks. All XML parsing in the application that + * touches user-controlled bytes (e.g. uploaded GPX files) MUST go through + * one of these helpers rather than calling {@link DocumentBuilderFactory#newInstance()} + * directly. + * + *

The hardening applied here disables DTDs, external entities, parameter + * entities, and external DTD loading, and enables the JAXP secure-processing + * feature. Together these defeat the standard XXE payloads (file disclosure + * via {@code SYSTEM "file:///..."}, billion laughs, SSRF via external entities). + */ +public final class SecureXmlFactories { + + private SecureXmlFactories() { + } + + /** + * Returns a {@link DocumentBuilderFactory} hardened against XXE. + * + * @param namespaceAware whether to enable namespace awareness + * @return a hardened factory + */ + public static DocumentBuilderFactory newDocumentBuilderFactory(boolean namespaceAware) { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setNamespaceAware(namespaceAware); + try { + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + } catch (ParserConfigurationException e) { + // Any underlying parser that does not support these features is + // unsafe — fail loudly rather than silently fall back. + throw new IllegalStateException("Failed to configure secure XML parser", e); + } + factory.setXIncludeAware(false); + factory.setExpandEntityReferences(false); + return factory; + } +} diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 06625e1..3a4e4e5 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -72,7 +72,9 @@ fitpub: # Security settings security: jwt: - secret: ${JWT_SECRET:change-this-secret-key-in-production-must-be-at-least-32-characters-long} + # JWT_SECRET must be set explicitly. The dev profile (application-dev.yml) provides a development default; + # production deployments without JWT_SECRET set will fail to start. + secret: ${JWT_SECRET:} expiration: 86400000 # 24 hours in milliseconds # Registration settings diff --git a/src/main/resources/static/css/fitpub.css b/src/main/resources/static/css/fitpub.css index 9b541b6..67d6242 100644 --- a/src/main/resources/static/css/fitpub.css +++ b/src/main/resources/static/css/fitpub.css @@ -1360,3 +1360,14 @@ h1 { color: #000000; font-weight: 900; } + +/* Hashtag links inside titles, descriptions, and detail pages */ +.hashtag-link { + color: #0d6efd; + text-decoration: none; + font-weight: 500; +} + +.hashtag-link:hover { + text-decoration: underline; +} diff --git a/src/main/resources/static/js/timeline.js b/src/main/resources/static/js/timeline.js index e0f27a6..d635ee2 100644 --- a/src/main/resources/static/js/timeline.js +++ b/src/main/resources/static/js/timeline.js @@ -9,6 +9,7 @@ const FitPubTimeline = { timelineType: 'public', searchText: '', dateFilter: '', + hashtagFilter: '', searchDebounceTimer: null, /** @@ -17,7 +18,45 @@ const FitPubTimeline = { */ init: function(type) { this.timelineType = type; + + // Read hashtag filter from URL query string + const params = new URLSearchParams(window.location.search); + const hashtagParam = params.get('hashtag'); + if (hashtagParam && /^\w+$/.test(hashtagParam)) { + this.hashtagFilter = hashtagParam; + } + this.setupSearchHandlers(); + this.renderHashtagFilterBadge(); + this.loadTimeline(0); + }, + + /** + * Show or hide the active hashtag filter badge + */ + renderHashtagFilterBadge: function() { + const badge = document.getElementById('hashtagFilterBadge'); + if (!badge) return; + + if (this.hashtagFilter) { + const label = badge.querySelector('#hashtagFilterLabel'); + if (label) label.textContent = '#' + this.hashtagFilter; + badge.classList.remove('d-none'); + } else { + badge.classList.add('d-none'); + } + }, + + /** + * Clear the active hashtag filter + */ + clearHashtagFilter: function() { + this.hashtagFilter = ''; + this.renderHashtagFilterBadge(); + // Update URL without reload + const url = new URL(window.location.href); + url.searchParams.delete('hashtag'); + window.history.replaceState({}, '', url); this.loadTimeline(0); }, @@ -68,6 +107,10 @@ const FitPubTimeline = { endpoint += `&search=${encodeURIComponent(this.searchText)}`; } + if (this.hashtagFilter) { + endpoint += `&hashtag=${encodeURIComponent(this.hashtagFilter)}`; + } + if (this.dateFilter) { // Only add if valid format const validation = this.validateDateFormat(this.dateFilter); @@ -166,18 +209,14 @@ const FitPubTimeline = {

${activity.isLocal - ? ` - ${this.escapeHtml(activity.title || 'Untitled Activity')} - ` - : ` - ${this.escapeHtml(activity.title || 'Untitled Activity')} - - ` + ? this.renderTitleLinkWithHashtags(activity.title, `/activities/${activity.id}`, 'activity-title-link', '') + : this.renderTitleLinkWithHashtags(activity.title, activity.activityUri || '#', 'activity-title-link', 'target="_blank"') + + (activity.isLocal ? '' : ' ') }
${activity.description - ? `

${this.escapeHtml(activity.description).substring(0, 200)}${activity.description.length > 200 ? '...' : ''}

` + ? `

${this.linkifyHashtags(activity.description.length > 200 ? activity.description.substring(0, 200) + '...' : activity.description)}

` : '' } @@ -562,6 +601,58 @@ const FitPubTimeline = { return div.innerHTML; }, + /** + * Escape text for safe HTML insertion AND turn #hashtags into links + * pointing to the public timeline filtered by that hashtag. + * @param {string} text - Text to process + * @returns {string} HTML-safe string with hashtag anchors + */ + linkifyHashtags: function(text) { + if (!text) return ''; + const escaped = this.escapeHtml(text); + return escaped.replace(/(^|\s)#(\w+)/g, (match, lead, tag) => + `${lead}#${tag}` + ); + }, + + /** + * Render a title that links to an activity, with embedded #hashtags + * linking instead to the public timeline filtered by that tag. + * Avoids invalid nested tags by rendering segments as siblings. + * @param {string} text - Title text + * @param {string} activityHref - Link target for non-hashtag portions + * @param {string} extraClass - Extra CSS classes for the activity link segments + * @param {string} extraAttrs - Extra HTML attributes for the activity link segments + * @returns {string} HTML + */ + renderTitleLinkWithHashtags: function(text, activityHref, extraClass, extraAttrs) { + const safeText = text || 'Untitled Activity'; + extraClass = extraClass || ''; + extraAttrs = extraAttrs || ''; + const wrapActivity = (chunk) => + chunk + ? `${chunk}` + : ''; + + const parts = []; + const regex = /(^|\s)#(\w+)/g; + let last = 0; + let m; + while ((m = regex.exec(safeText)) !== null) { + // Text before the hashtag (and the leading whitespace) goes to activity + const before = safeText.substring(last, m.index) + m[1]; + if (before) parts.push(wrapActivity(this.escapeHtml(before))); + const tag = m[2]; + parts.push( + `#${this.escapeHtml(tag)}` + ); + last = m.index + m[0].length; + } + const tail = safeText.substring(last); + if (tail) parts.push(wrapActivity(this.escapeHtml(tail))); + return parts.join(''); + }, + /** * Render indoor activity placeholder with emoji * @param {HTMLElement} element - Container element diff --git a/src/main/resources/templates/activities/detail.html b/src/main/resources/templates/activities/detail.html index cc98442..4fbdbdb 100644 --- a/src/main/resources/templates/activities/detail.html +++ b/src/main/resources/templates/activities/detail.html @@ -519,7 +519,7 @@ function renderActivity(activity) { // Header - document.getElementById('activityTitle').textContent = activity.title || 'Untitled Activity'; + document.getElementById('activityTitle').innerHTML = linkifyHashtags(activity.title || 'Untitled Activity'); document.getElementById('activityType').textContent = activity.activityType; document.getElementById('activityType').className = `activity-type-badge activity-type-${activity.activityType.toLowerCase()}`; // Format date with timezone awareness @@ -570,7 +570,7 @@ // Description if (activity.description) { - document.getElementById('activityDescription').textContent = activity.description; + document.getElementById('activityDescription').innerHTML = linkifyHashtags(activity.description); } else { document.getElementById('activityDescription').style.display = 'none'; } @@ -1721,6 +1721,14 @@ return div.innerHTML; } + function linkifyHashtags(text) { + if (!text) return ''; + const escaped = escapeHtml(text); + return escaped.replace(/(^|\s)#(\w+)/g, (match, lead, tag) => + `${lead}#${tag}` + ); + } + // Delete functionality document.getElementById('deleteBtn').addEventListener('click', function() { const modal = new bootstrap.Modal(document.getElementById('deleteModal')); diff --git a/src/main/resources/templates/activities/list.html b/src/main/resources/templates/activities/list.html index 1b9d76a..9493ae3 100644 --- a/src/main/resources/templates/activities/list.html +++ b/src/main/resources/templates/activities/list.html @@ -152,9 +152,7 @@
- - ${escapeHtml(activity.title || 'Untitled Activity')} - + ${renderTitleLinkWithHashtags(activity.title, `/activities/${activity.id}`)}

@@ -181,7 +179,7 @@ ${activity.visibility}

- ${activity.description ? `

${escapeHtml(activity.description).substring(0, 150)}${activity.description.length > 150 ? '...' : ''}

` : ''} + ${activity.description ? `

${linkifyHashtags(activity.description.length > 150 ? activity.description.substring(0, 150) + '...' : activity.description)}

` : ''}
@@ -332,6 +330,35 @@ div.textContent = text; return div.innerHTML; } + + function linkifyHashtags(text) { + if (!text) return ''; + const escaped = escapeHtml(text); + return escaped.replace(/(^|\s)#(\w+)/g, (match, lead, tag) => + `${lead}#${tag}` + ); + } + + function renderTitleLinkWithHashtags(text, activityHref) { + const safeText = text || 'Untitled Activity'; + const wrap = (chunk) => chunk + ? `${chunk}` + : ''; + const parts = []; + const regex = /(^|\s)#(\w+)/g; + let last = 0; + let m; + while ((m = regex.exec(safeText)) !== null) { + const before = safeText.substring(last, m.index) + m[1]; + if (before) parts.push(wrap(escapeHtml(before))); + const tag = m[2]; + parts.push(`#${escapeHtml(tag)}`); + last = m.index + m[0].length; + } + const tail = safeText.substring(last); + if (tail) parts.push(wrap(escapeHtml(tail))); + return parts.join(''); + } }); diff --git a/src/main/resources/templates/timeline/public.html b/src/main/resources/templates/timeline/public.html index 4176050..5ea3ece 100644 --- a/src/main/resources/templates/timeline/public.html +++ b/src/main/resources/templates/timeline/public.html @@ -62,6 +62,18 @@
+ + +