Leerer Synchronized-Block
Beschreibung
Leerer Synchronized-Block ist eine Codequalitätsschwäche in Java, bei der ein synchronized-Block keine ausführbaren Anweisungen enthält. Synchronisation wird verwendet, um sicherzustellen, dass nur ein Thread gleichzeitig auf einen kritischen Abschnitt zugreifen kann und gemeinsame Ressourcen vor gleichzeitiger Modifikation geschützt werden. Ein leerer synchronized-Block erwirbt eine Sperre, tut nichts und gibt sie frei - ohne tatsächlichen Schutz für irgendwelche gemeinsamen Daten zu bieten. Dieses Muster tritt typischerweise auf, wenn Entwickler Code innerhalb eines synchronized-Blocks auskommentieren ohne die Synchronisation selbst zu entfernen, oder wenn Code falsch refaktoriert wird und rudimentäre Synchronisationslogik zurückbleibt.
Risiko
Leere synchronized-Blöcke erzeugen mehrere Probleme. Während der Code Synchronisation durchzuführen scheint, bietet er tatsächlich keine Thread-Sicherheit für nachfolgende Operationen. Entwickler können fälschlicherweise glauben, ihr Code sei thread-sicher, wenn er es nicht ist. Der leere Block verursacht trotzdem Synchronisationsoverhead und unnötige Leistungsverschlechterung. Das Muster weist auf potenzielle Fehler hin - entweder wurde kritischer Code versehentlich entfernt, oder die Synchronisation selbst hätte entfernt werden sollen. In Sicherheitskontexten können Race-Conditions, die die Synchronisation verhindern sollte, noch existieren und zu Datenkorruption oder inkonsistentem Zustand führen.
Lösung
Untersuchen Sie die Absicht hinter leeren synchronized-Blöcken. Wenn der synchronisierte Code entfernt wurde aber Synchronisation noch benötigt wird, fügen Sie die angemessenen thread-sicheren Operationen hinzu. Wenn der geschützte Code nicht mehr benötigt wird, entfernen Sie den gesamten synchronized-Block, um unnötigen Sperrerwerb zu eliminieren. Verwenden Sie Code-Review und statische Analysewerkzeuge, um leere synchronized-Blöcke zu erkennen. Beim Refactoring von Code stellen Sie sicher, dass synchronized-Blöcke und ihre Inhalte zusammen behandelt werden. Dokumentieren Sie den Zweck der Synchronisation, um versehentliches Entfernen von kritischem Code zu verhindern.
Häufige Auswirkungen
| Auswirkung | Details |
|---|---|
| Sonstige | Bereich: Sonstige Qualitätsverschlechterung - Der Code suggeriert Synchronisation, bietet aber keine, was auf Fehler oder Code-Verfall hindeutet. |
| Integrität | Bereich: Integrität Anwendungsdaten modifizieren - Race-Conditions, die hätten verhindert werden sollen, können noch auftreten und gemeinsame Daten korrumpieren. |
Beispielcode
Verwundbarer Code
// Verwundbar: Leerer synchronized-Block
public class VulnerableEmptySync {
private int sharedCounter = 0;
public void incrementCounter() {
synchronized(this) {
// Leer - Synchronisation tut nichts!
}
// Dieser Code läuft AUSSERHALB des synchronized-Blocks
// und ist NICHT thread-sicher!
sharedCounter++;
}
}
// Verwundbar: Code entfernt, sync-Block zurückgelassen
public class VulnerableCommentedOut {
private List<String> items = new ArrayList<>();
public void processItem(String item) {
synchronized(items) {
// Code war hier, wurde aber beim Refactoring auskommentiert:
// items.add(item);
// processInternal(item);
}
// Entwickler dachte, dies sei synchronisiert, aber ist es nicht!
items.add(item); // Race-Condition!
}
}
// Verwundbar: Fehlplatzierte Synchronisation
public class VulnerableMisplaced {
private volatile boolean flag = false;
private String data = null;
public void setData(String value) {
// Verwundbar: Leerer sync-Block vor kritischem Abschnitt
synchronized(this) {
// Oops, vergessen die Zuweisung hier zu platzieren
}
// Zuweisung ist NICHT synchronisiert
data = value;
flag = true;
}
public String getData() {
synchronized(this) {
// Leerer Block
}
// Lesen ist NICHT synchronisiert - kann inkonsistenten Zustand sehen
if (flag) {
return data;
}
return null;
}
}
// Verwundbar: Ergebnis unvollständigen Refactorings
public class VulnerableRefactored {
private Map<String, Object> cache = new HashMap<>();
// Ursprünglicher Code hatte Cache-Operationen im synchronized-Block
// Nach Wechsel zu ConcurrentHashMap vergaß Entwickler sync-Block zu entfernen
public Object getFromCache(String key) {
synchronized(cache) {
// War: return cache.get(key);
// Jetzt leer nach Wechsel zu ConcurrentHashMap
}
// Neuer Code mit ConcurrentHashMap
// Der synchronized-Block oben ist nutzlos
return cache.get(key);
}
}
// Verwundbar: Synchronisation auf falschem Objekt
public class VulnerableWrongLock {
private final Object lock = new Object();
private int value;
public void setValue(int v) {
synchronized(lock) {
// Leerer Block - sollte value-Zuweisung unten schützen
}
// Nicht geschützt!
value = v;
}
public int getValue() {
synchronized(new Object()) {
// Synchronisierung auf neuem Objekt jedes Mal - nutzlos!
}
return value;
}
}
Lösungscode
// Behoben: Synchronized-Block mit tatsächlichem Inhalt
public class SecureEmptySync {
private int sharedCounter = 0;
public void incrementCounter() {
synchronized(this) {
// Behoben: Kritische Operation ist innerhalb des synchronized-Blocks
sharedCounter++;
}
}
public int getCounter() {
synchronized(this) {
return sharedCounter;
}
}
}
// Behoben: Vollständige Entfernung unnötiger Sync oder ordnungsgemäße Einbeziehung
public class SecureProperSync {
private List<String> items = new ArrayList<>();
// Option 1: Sync-Block entfernen wenn nicht benötigt
public void processItemNoSync(String item) {
// Wenn Thread-Sicherheit anderswo behandelt wird, Sync ganz entfernen
items.add(item);
}
// Option 2: Code in Sync-Block einschließen
public void processItemSynced(String item) {
synchronized(items) {
// Behoben: Code innerhalb des synchronized-Blocks
items.add(item);
processInternal(item);
}
}
// Option 3: Concurrent-Collection stattdessen verwenden
private List<String> concurrentItems =
Collections.synchronizedList(new ArrayList<>());
public void processItemConcurrent(String item) {
// Kein sync-Block nötig für add
concurrentItems.add(item);
}
}
// Behoben: Ordnungsgemäße Synchronisationsplatzierung
public class SecureProperPlacement {
private volatile boolean ready = false;
private String data = null;
private final Object lock = new Object();
public void setData(String value) {
synchronized(lock) {
// Behoben: Alle verwandten Operationen im Sync-Block
data = value;
ready = true;
}
}
public String getData() {
synchronized(lock) {
// Behoben: Leseoperation im Sync-Block
if (ready) {
return data;
}
return null;
}
}
}
// Behoben: Vollständiges Refactoring - rudimentäre Synchronisation entfernen
public class SecureRefactored {
// Behoben: ConcurrentHashMap verwenden - keine sync-Blöcke nötig
private Map<String, Object> cache = new ConcurrentHashMap<>();
public Object getFromCache(String key) {
// Kein synchronized-Block - ConcurrentHashMap behandelt es
return cache.get(key);
}
public void putInCache(String key, Object value) {
cache.put(key, value);
}
// Wenn atomare Operationen benötigt werden:
public Object computeIfAbsent(String key) {
return cache.computeIfAbsent(key, k -> createValue(k));
}
}
// Behoben: Höherstufige Nebenläufigkeits-Utilities verwenden
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
public class SecureReadWriteLock {
private final ReadWriteLock lock = new ReentrantReadWriteLock();
private Map<String, String> data = new HashMap<>();
public String read(String key) {
lock.readLock().lock();
try {
// Behoben: Tatsächliche Leseoperation innerhalb der Sperre
return data.get(key);
} finally {
lock.readLock().unlock();
}
}
public void write(String key, String value) {
lock.writeLock().lock();
try {
// Behoben: Tatsächliche Schreiboperation innerhalb der Sperre
data.put(key, value);
} finally {
lock.writeLock().unlock();
}
}
}
// Behoben: AtomicInteger statt synchronized-Blöcke verwenden
import java.util.concurrent.atomic.AtomicInteger;
public class SecureAtomicCounter {
private AtomicInteger counter = new AtomicInteger(0);
public void increment() {
// Kein synchronized-Block nötig - atomare Operation
counter.incrementAndGet();
}
public int get() {
return counter.get();
}
public int incrementAndGet() {
return counter.incrementAndGet();
}
}
// Muster: Synchronized-Blöcke dokumentieren
public class DocumentedSync {
private final Object dataLock = new Object();
private String sensitiveData;
private int accessCount;
/**
* Aktualisiert sensible Daten atomar mit Zugriffszählung.
* Synchronisation erforderlich um Atomarität von
* Datenaktualisierung und Zählerinkrement sicherzustellen.
*/
public void updateData(String newData) {
synchronized(dataLock) {
// KRITISCH: Beide Operationen müssen atomar sein
sensitiveData = newData;
accessCount++;
// ENDE KRITISCHER ABSCHNITT
}
}
}
CVE-Beispiele
Keine spezifischen CVEs werden dieser CWE üblicherweise zugeordnet, da sie primär die Codequalität und Zuverlässigkeit betrifft statt direkter Sicherheitsschwachstellen.
Referenzen
- MITRE Corporation. "CWE-585: Empty Synchronized Block." https://cwe.mitre.org/data/definitions/585.html
- Java Concurrency in Practice von Brian Götz.
- FindBugs. "ESync: Empty synchronized block."