# Mangelnde Qualitätssicherung
**URL:** https://forum.instar.com/t/mangelnde-qualitaetssicherung/30494
**Category:** Verbesserungen / Ideen / Vorschläge
**Tags:** entwickler
**Created:** [31. Dezember 2024 um 15:18 UTC](https://forum.instar.com/t/mangelnde-qualitaetssicherung/30494 "2024-12-31T15:18:52Z")
**Posts on this page:** 4
**Page:** 1
### Author:  [@DrRosental](https://forum.instar.com/u/DrRosental)
#### Post date: [31. Dezember 2024 um 15:18 UTC](https://forum.instar.com/t/mangelnde-qualitaetssicherung/30494/1 "2024-12-31T15:18:52Z")
Dieser Beitrag gilt einzig und allein dem Ziel der Qualitätssteigerung. Ich bin kein Spezialist - und deshalb erschüttert es mich, dass ich mich in der Lage sehe, in offenen Quelltexten von INSTAR Fehler und Sicherheitslücken zu entdecken. Ich würde dieses Code-Review auch nicht schreiben, wenn die INSTAR-Produkte nicht den Sicherheitssektor beträfen. Das ist der Sektor, in dem es so sehr darauf ankommt, pingelig und pedantisch zu sein!
Ich betrachte hier den von INSTAR im [Download-Bereich](https://wiki.instar.com/dl/Developer/) veröffentlichten WebSocket-Client (html\_ws\_client.zip) für die Einbindung eines Livestreams in eigene Webseiten. Wenn ich diesen Code lese, mag ich gar nicht daran denken, dass das Backend möglicherweise genauso schnell und unbedacht entwickelt wird; dieser Quelltext eignet sich hervorragend, um Studierenden aufzuzeigen, was man alles falsch machen kann. Wenn ich mir den Code anschaue, habe ich das Gefühl, das INSTAR keinerlei Qualitätskriterien für den entwickelten Code festlegt. Im Gegenteil: der Code sieht aus, als wäre er in einem Wahnsinns-Tempo geschrieben, was ihn leider unwahrscheinlich schlecht macht. Insbesondere kann ich mir nicht vorstellen, dass hier überhaupt mehr als ein Paar Augen an der Entwicklung beteiligt war. Noch einmal zur Sicherheit: Ich will niemanden persönlich angreifen oder gar verletzen, sondern mache mir diese Mühe nur, um zu helfen.
Grundsätzlich: Wenn man heute JavaScript schreibt, sollte man `var` vermeiden und statt dessen `const` verwenden, wo möglich, und nur dort `let` verwenden, wo nötig.
index.html: Zeile 35:
```auto
```
wsclient.js: Zeilen 1-7:
```auto
// Add your camera configuration here
var ws_protocol = "ws";
var ws_hostname = "192.168.2.120";
var ws_port = "80";
var ws_endpoint = "/ws";
var cam_username = "admin";
var cam_password = "instar";
```
Wieso werden äußerst sensible Daten auf globaler Ebene abgelegt? Das ist ein Sicherheitsrisiko! Sobald INSTAR-Kunden weitere Skripte über `script`-Tags einbinden, können die Zugangsdaten an beliebige Server versendet werden - und nein, die [Same-Origin-Policy](https://de.wikipedia.org/wiki/Same-Origin-Policy) unserer Webbrowser schützt uns nicht, denn es genügt ein dynamisch erzeugtes `img`-Tag mit einem `src`-Attribut à la `https://uglyserver.example.com/?STOLEN_DATA` - und schon sind die Daten erfolgreich abgefischt! Einfache Methoden, um diesem Problem entgegenzuwirken, sollten lokale Namensräume (oder insbesondere Module) sein - aber auch das will wohlüberlegt sein!
wsclient.js: Zeilen 43-51:
```auto
if (codec_mime.value = 0) {
var mimeCodec = 'video/mp4; codecs="avc1.4D001E, mp4a.40.2"';
} else if (codec_mime.value = 1) {
var mimeCodec = 'video/mp4; codecs="avc1.4D001E"';
} else if (codec_mime.value = 2) {
var mimeCodec = 'video/mp4; codecs="hev1.2.4.L120.B0, mp4a.40.2"';
} else {
var mimeCodec = 'video/mp4; codecs="hev1.2.4.L120.B0"';
}
```
Die Variable `codec_mime` ist offenbar vorgesehen, um ganze Zahlen zu speichern. Warum wird hier `codec_mime.value` angeschaut? Das ergibt keinen Sinn. Davon abgesehen suggerieren diese Zeilen, dass der Wert von `codec_mime.value` überprüft würde, was leider nicht passiert. Da die Bedingungen hier **Zuweisungen** (nur ein Gleichheitszeichen statt zweien! Das ist ein Fehler, der darauf hinweist, dass der Code überhaupt nicht getestet wurde!) sind, ist der initiale Wert von `codec_mime.value` gänzlich unerheblich; nach Zeile 51 gilt **immer** , dass `mimeCodec == 'video/mp4; codecs="avc1.4D001E"'`. Der Benutzer kann hier also effektiv gar nichts auswählen!
wsclient.js: Zeile 106:
```auto
if ((ws_protocol === '') || (ws_hostname === '') || (ws_port === '') || (ws_endpoint === '') ||(cam_username === '') || (cam_password === '')) {
```
Was soll diese Überprüfung nutzen? Das Backend sollte (hoffentlich!) nicht darauf angewiesen sein, dass diese Zeichenketten alle nicht-leer sind. Wenn sie es nicht sind, kommt eben keine Verbindung zustande. Weg damit - schlankerer Code ist besserer Code!
wsclient.js: Zeile 126:
```auto
webSocketURL = protocol + "://" + username + ":" + password + "@" + hostname + ":" + port + endpoint;
```
So kann man die `webSocketURL` beim besten Willen nicht konstruieren. Wenn `username` und/oder `password` Sonderzeichen enthalten, gibt es schnell Schwierigkeiten: einerseits wird die Verbindung nicht funktionieren - andererseits werden ggf. Teile der Zugangsdaten als hostname interpretiert, was wiederum unmittelbar eine Sicherheitslücke bedeutet! Sowohl `username` als auch `password` **müssen** hier unbedingt mit `encodeURIComponent()` kodiert werden, bevor sie in die `webSocketURL` eingebaut werden!
Eine Start-Stop-Automatik ist in meinen Augen insgesamt wenig sinnvoll. Besser wäre ein automatisch startender Viewer. Warum liefert die Kamera einen WebSocket-Viewer nicht passwortgeschützt über ihren HTTP-Server aus? Mit Hilfe von Reverse-Proxys kann man die auch elegant und sicher nach „außen“ weiterleiten, wenn man das möchte.
Abschließend hoffe ich sehr, dass INSTAR hier die Größe besitzt, um Einsicht zu zeigen. Quelltexte wie diese sind einfach katastrophal und sollten nicht an Endkunden ausgeliefert werden (die häufig von Technik keine Ahnung haben und diese unverändert einsetzen!). Ein guter Vorsatz für das neue Jahr sollte sein, dass INSTAR alle Quelltexte (etwa auf GitHub) offenlegt. Niemand kauft INSTAR-Kameras, weil die Software so einzigartig und unnachahmlich wäre, sondern ganz einfach, weil man auf die Hardware angewiesen ist - von der ich sehr viel halte! Eine Quellcode-Offenlegung wird sicher viele Sicherheitslücken offenbaren - und das ist großartig, weil sie dann geschlossen werden können!
---
### Author:  [@Raphael](https://forum.instar.com/u/Raphael)
#### Post date: [31. Dezember 2024 um 20:15 UTC](https://forum.instar.com/t/mangelnde-qualitaetssicherung/30494/2 "2024-12-31T20:15:50Z")
> Wenn ich diesen Code lese, mag ich gar nicht daran denken, dass das Backend möglicherweise genauso schnell und unbedacht entwickelt wird
Snippets aus dem Wiki und Forum, wie das oben genannte, haben in der Regel nichts mit der Entwicklung der Kamerasoftware zu tun und werden teilweise von der Community bereitgestellt. Das WS-Snippet sollte auch auf GitHub zu finden sein. Unser Wiki-Manager @m.polinowski kann sicherlich einen Link dazu teilen. Am besten dort dann einen Pull-Request zu erstellen.
Unabhängig davon stimme ich aber zu, dass Code, der im Namen von INSTAR veröffentlicht wird, überprüft werden sollte. Wir werden das besprechen.
> Warum liefert die Kamera einen WebSocket-Viewer nicht passwortgeschützt über ihren HTTP-Server aus?
Spricht nichts dagegen. Wenn der Wunsch da ist, setzen wir uns das auf jeden Fall auf die Liste.
> Ein guter Vorsatz für das neue Jahr sollte sein, dass INSTAR alle Quelltexte (etwa auf GitHub) offenlegt.
Für alles was gefährlich ist (Kommunikation und Verschlüsselung) greifen wir auf etablierte Open-Source-Libraries zurück. Dabei sind wir regelmäßig auch als Contributor aktiv und haben bereits zur Entdeckung und Behebung verschiedener Probleme beigetragen. Eine vollständige Offenlegung der Firmware ist für uns kaum möglich, da wir uns mit einigen Features dann doch von der Konkurrenz abheben. So ist zum Beispiel HomeKit und MQTT für manche ein Kaufgrund.
---
### Author:  [@DrRosental](https://forum.instar.com/u/DrRosental)
#### Post date: [1. Januar 2025 um 13:27 UTC](https://forum.instar.com/t/mangelnde-qualitaetssicherung/30494/3 "2025-01-01T13:27:19Z")
Ein frohes neues Jahr und vielen Dank für die freundliche Antwort! Für potenziell beunruhigte Leser will ich einmal festhalten, dass es sich in dem Quelltext, den ich kritisiert habe, in der Tat um solchen handelt, der **nicht** auf den INSTAR-Kameras installiert ist und von INSTAR optional zur Verfügung gestellt wird. Ich bin ein glücklicher INSTAR-Kunde und bleibe vermutlich einer - insbesondere auch, weil mir der Umgang mit Kritik wie meiner sehr gut gefällt!
Ich freue mich auf jeden Fall, wenn der WebSocket-Viewer auf die Liste kommt. Besonders toll wäre, wenn der auch mit iOS+Safari nutzbar wäre. Vermutlich gibt es da allerdings Schwierigkeiten mit dem Encoding - da kenne ich mich nicht aus aber mir scheint, dass der eingebettete Viewer in der Administrationsoberfläche unter iOS+Safari deshalb über MJPEG läuft?
Das Abheben von der Konkurrenz ist (leider) ein valider Grund, um Quelltexte geschlossen zu halten - allerdings bin ich nicht einverstanden, was den Rückgriff auf bestehende Software angeht: dabei kann man beliebig schlimme Fehler machen. Eine tolles Hybridverfahren könnte entstehen, indem einmal genau herausgearbeitet wird, welche Teile der Software geöffnet werden können - und welche verschlossen bleiben sollten bzw. müssen.
---
### Author:  [@system](https://forum.instar.com/u/system)
#### Post date: [3. Januar 2025 um 13:27 UTC](https://forum.instar.com/t/mangelnde-qualitaetssicherung/30494/4 "2025-01-03T13:27:32Z")
Dieses Thema wurde automatisch 2 Tage nach der letzten Antwort geschlossen. Es sind keine neuen Antworten mehr erlaubt.