WBCE CMS Forum

webbird

Administrator

Planung - NWI (6.x) 5.1

Könnte mir bitte jemand helfen, die aufgelaufenen Anforderungen und insbesondere Probleme mit der letzten 5.x Version zu sammeln? Bevorzugt als Issues bei GitHub.

Basis ist die Version 5.0.30 von GitHub. Wer noch darauf basierende Änderungen herumliegen hat, bitte entweder als PR bei GitHub einstellen, oder in diesen Thread hängen.

Danke!

https://github.com/WBCE/News-with-Images/tree/v6.0

Edit 2026-04-13: Aufgrund der bisher sehr geringen Rückmeldungen wird es dann doch eher eine 5.1. Ich sehe da kein "Killerfeature", das ein Major Release rechtfertigt.

Was ich von woanders als Wunsch mitgenommen habe:

• Pro Sektion einstellbarer Fallback auf "Standardbild", wenn es keine Beitragsbilder gibt

• https://github.com/WBCE/News-with-Images/issues/102

• Beiträge kopieren inkl. Stichworte funktioniert nicht https://github.com/WBCE/News-with-Images/issues/103 --- gefixt

Last edited by webbird (13.04.2026 18:17:56)

---

Ich habe eine Amazon-Wishlist. Oder spende an das Projekt.
Ich kann, wenn ich will, aber wer will, dass ich muss, kann mich mal

webbird

Administrator

Re: Planung - NWI (6.x) 5.1

# Code Review: `modules/news_img` (v5.0.30)

## Overview
A WBCE CMS page module for creating news pages with image galleries, tagging, grouping, RSS feeds, prev/next navigation, and import from other modules. ~7,000 lines across ~50 files, with the bulk in `functions.inc.php` (2,615 lines).

---

## Critical: Security Issues

### ~~1. PHP Object Injection via `unserialize()` of user input~~ FIXED
~~**`save_post.php:91-101`, `functions.inc.php:692,1005`**~~

Replaced `serialize()`/`unserialize()` with a simple pipe-delimited integer string (`g|s|p`). The group/section/page IDs in the group select dropdown are now transported as e.g. `5|12|3` instead of serialized PHP arrays. Changed in `group_select.phtml`, `save_post.php`, `functions.inc.php` (2x), and `manage_posts.php`.

### ~~2. SQL Injection via `$_GET['g']` in `view.php:68`~~ FIXED

Alle Integer-`$_GET`-Parameter im Modul auf `filter_input()` mit `FILTER_VALIDATE_INT` und Range-Validierung umgestellt. Betroffen: `g` (min:1), `p` (min:0/1), `m` (1–12), `y` (1970–2100), `method` (0–1), `value` (0–1) in `functions.inc.php`, `view.php` und `activate_post.php`. `is_numeric()`-Guards und rohe `$_GET`-Zugriffe entfernt.

### ~~3. SQL Injection in `rss.php:70-76`~~ FIXED

~~`$time_check_str= "... && (`published_until`..."  // uses && instead of AND`~~
~~`$query = "... WHERE `group_id`=".$group_id." && `section_id` = ".$section_id." ...";`~~

Replaced all `&&` with `AND` in query strings (`rss.php:70,73,75`). Also fixed `TIME()` → `time()` (`rss.php:55`) — `TIME()` was undefined in PHP and caused a fatal error. `$group_id`, `$section_id` and `$t` are all integer-safe via `intval()`/`time()`.

### ~~4. XSS via tag output in `view.php:92`~~ FIXED

Tag-Namen werden jetzt in Link-Text mit `htmlspecialchars(ENT_QUOTES|ENT_HTML5)` und im URL-Parameter mit `urlencode()` ausgegeben. Geändert in `view.php:94` und `functions.inc.php:1360`.

### ~~5. Unsafe `unlink()` without path validation~~ FIXED

Dateinamen aus der DB werden jetzt mit `basename()` bereinigt (verhindert Path-Traversal wie `../../config.php`). `@unlink()` ersetzt durch `file_exists()` + `unlink()`. Geändert in `modify_post.php:54,77–78`.

---

## High: Logic & Correctness Bugs

### ~~6. Bug in `mod_nwi_post_show()` at `functions.inc.php:1110`~~ FIXED
Dead `is_array($post_id)` block und fehlerhafter `$post_id['group_id']`-Zugriff entfernt. `$gid` wird jetzt korrekt aus `$post['group_id']` gelesen.

### ~~7. Undefined variable `$position` at `functions.inc.php:1526`~~ FIXED
`$position > 0`-Bedingung entfernt. Da `$filter_p` bereits `min_range: 1` hat, ist die Bedingung implizit erfüllt.

### ~~8. Undefined variable `$tagList` at `functions.inc.php:2218`~~ FIXED
`'taglist' => $tagList` aus dem Options-Array entfernt. Default in `mod_nwi_get_news_items()` ist bereits `null`.

### ~~9. Variable name case mismatch at `functions.inc.php:2454`~~ FIXED
`$tagListArray = array()` → `$taglistArray = array()` (lowercase l), konsistent mit allen weiteren Verwendungen.

### ~~10. Bug in `mod_nwi_get_news_items()` at `functions.inc.php:2443`~~ FIXED
`$row['content_short']` → `$post['content_short']` (korrekter Loop-Variablenname).

### ~~11. `$posts` used before initialization at `functions.inc.php:2470`~~ FIXED
`$posts = []` vor dem `while`-Loop hinzugefügt.

---

## Medium: Code Quality

### ~~12. Inconsistent `$_GET` handling for `post_id`~~ FIXED

Alle `checkIDKEY()`/`getIDKEY()` + `version_compare(WB_VERSION, '2.8.3', '>')` Blöcke entfernt (17 Stellen in 9 Dateien). IDs werden jetzt direkt per `filter_input(FILTER_VALIDATE_INT)` gelesen bzw. als rohe IDs weitergereicht. Betroffen: `modify_post.php`, `delete_post.php`, `activate_post.php`, `add_post.php`, `move_up.php`, `move_down.php`, `save_post.php`, `functions.inc.php`, `uploader/upload.php`, `ajax/ajax_dragdrop.php`.

### 13. Duplicate code blocks
The index.php protection content (header redirect) is copy-pasted 4+ times in `install.php` and `functions.inc.php`. The `mod_nwi_img_makedir()` function exists to address this, but `install.php` still uses inline versions.

### ~~14. Global variable pollution~~ WON'T FIX
`global $database` / `global $admin` ist WBCE-Konvention — jedes Modul und der Core nutzen dieses Pattern. Eine Änderung würde `news_img` inkonsistent mit dem Rest von WBCE machen bei unverhältnismäßig hohem Aufwand.

### ~~15. Missing `$TEXT` global in `mod_nwi_posts_getall()`~~ FIXED
`global $TEXT` zur `global`-Deklaration in `mod_nwi_posts_getall()` hinzugefügt.

### ~~16. `.bak` files in module directory~~ WON'T FIX
~~`functions.inc.php.bak`, `info.php.bak`, `rss.php.bak` are present. These should not be distributed and may leak source code on misconfigured servers.~~

---

## Medium: Performance

### ~~17. N+1 query pattern in `mod_nwi_posts_render()`~~ FIXED
~~At line 1350, `mod_nwi_get_tags_for_post()` is called inside a loop over all posts. Each call runs a 4-table JOIN query. For a page with 20 posts, that's 20 additional queries.~~

`mod_nwi_get_tags_for_posts()` (Batch-Funktion) hinzugefügt. In `mod_nwi_posts_render()` wird jetzt einmalig vor dem Loop alle Tags per Batch geladen (`$tags_by_post = mod_nwi_get_tags_for_posts(array_column($posts, 'post_id'))`), danach nur noch Array-Lookup.

### ~~18. `mod_nwi_settings_get()` called redundantly~~ FIXED
~~Called multiple times for the same `$section_id` within a single page load (e.g., in `mod_nwi_post_process`, `mod_nwi_img_get_by_post`, `mod_nwi_posts_getall`, etc.). Should be cached in a static variable.~~

### ~~19. `mod_nwi_users_get()` fetches ALL users~~ WON'T FIX
~~At line 299, this queries every user in the system. For large installations this is wasteful when only a handful of users author news posts.~~

---

## Low: Style & Maintenance

- **Mixed indentation**: tabs vs. spaces inconsistent throughout
- **Dead code**: Multiple `if(defined('WB_VERSION'))` version checks for versions that no longer exist
- **German comments** mixed with English: `//update Bildbeschreibungen der tabelle`, `// wichtig fuer link "weiterlesen"`
- **Missing return types** on most functions despite PHP 7+ being required
- **`extract()` usage** in `search.php:17` and `functions.inc.php:2263` - makes code harder to follow and can introduce variable injection

---

## Summary

| Severity | Count |
|----------|-------|
| Critical (Security) | 4 (4 fixed) |
| High (Logic Bugs) | 6 (6 fixed) |
| Medium (Quality/Perf) | 7 (4 fixed, 2 won't fix) |
| Low (Style) | 5 |

**Top priorities:**
1. ~~Replace `unserialize()` on user input~~ FIXED - replaced with pipe-delimited integers
2. ~~Sanitize integer GET parameters~~ FIXED - all integer `$_GET` params use `filter_input(FILTER_VALIDATE_INT)` with range constraints
3. ~~Fix SQL `&&` → `AND` and `TIME()` → `time()` in `rss.php`~~ FIXED
4. ~~Escape/sanitize tag output to prevent XSS~~ FIXED - `htmlspecialchars()` + `urlencode()` auf Tag-Ausgabe
4. ~~Fix the undefined variable bugs~~ FIXED (#6–#11)
5. ~~Remove `.bak` files from distribution~~ WON'T FIX (#16)

Last edited by webbird (13.04.2026 17:56:25)

---

Ich habe eine Amazon-Wishlist. Oder spende an das Projekt.
Ich kann, wenn ich will, aber wer will, dass ich muss, kann mich mal

webbird

Administrator

Re: Planung - NWI (6.x) 5.1

Es gibt wieder ein paar Überarbeitungen. Aktuell ist das Modul auch noch auf WBCE 1.6.x lauffähig, da die Umstellung auf die neuen PDO-Funktionen noch aussteht.

Neues Feature
Man kann in den Einstellungen ein vorhandenes Beitragsbild als Standardbild einstellen. (Bisher: Fallback auf "nopic.png") Alternativ kann man auch gezielt ein Bild hochladen. Das Bild wird mit einem festen Namen gespeichert - analog zum Gruppenbild -, so dass es eindeutig der Sektion zugeordnet ist / werden kann. (Z.B. per FTP kann man das Standardbild der Sektion 4711 am Dateinamen erkennen.)

Fixes
Neben einer Reihe von Bugfixes (die z.T. eine Folge der oben aufgeführten Änderungen waren) habe ich auch die "Bild-Kaskade" und die Darstellung der mitgelieferten Views angepaßt. Wer sich die Bugfixes im Detail angucken will, kann bei GitHub in die Liste der Commits gucken. (https://github.com/WBCE/News-with-Images/commits/v6.0/)

In der Listenansicht der Beiträge wird jetzt mit folgender Kaskade gearbeitet:

Prio 1: Das im Beitrag explizit angegebene Bild
Prio 2: Das in der Gruppe angegebene Bild
Prio 3: Das Standardbild aus den Einstellungen (bisher: immer nopic.png)

Last edited by webbird (13.05.2026 18:20:19)

---

Ich habe eine Amazon-Wishlist. Oder spende an das Projekt.
Ich kann, wenn ich will, aber wer will, dass ich muss, kann mich mal