Fatale sur `autoriser` appelé avec un identifiant d’auteur inexistant #5259

Closed
opened 3 weeks ago by marcimat · 3 comments
marcimat commented 3 weeks ago
Owner

Exemple depuis autoriser_auteur_modifier_dist qui appelle

autoriser('webmestre', '', 0, 1234) se demandant si l’auteur à modifier a des droits de webmestre… mais si 1234 est inexistant, on obtient une fatale car $qui devient null (issu du sql_fetsel dans autoriser_dist), transmis ensuite à autoriser_webmestre_dist, qui attend un type array $qui

probablement cela pourrait atténuer déjà dans autoriser_dist

	} elseif (is_numeric($qui)) {
		$qui = sql_fetsel('*', 'spip_auteurs', 'id_auteur=' . $qui);
+       if (!$qui) {
+			return false;
+       }
	}

Mais ça ne serait pas testé par autoriser_exception qui est plus loin…

Exemple depuis autoriser_auteur_modifier_dist qui appelle `autoriser('webmestre', '', 0, 1234)` se demandant si l’auteur à modifier a des droits de webmestre… mais si 1234 est inexistant, on obtient une fatale car `$qui` devient null (issu du `sql_fetsel` dans autoriser_dist), transmis ensuite à autoriser_webmestre_dist, qui attend un type `array $qui` probablement cela pourrait atténuer déjà dans autoriser_dist ```diff } elseif (is_numeric($qui)) { $qui = sql_fetsel('*', 'spip_auteurs', 'id_auteur=' . $qui); + if (!$qui) { + return false; + } } ``` Mais ça ne serait pas testé par autoriser_exception qui est plus loin…
Poster
Owner
diff --git a/ecrire/inc/autoriser.php b/ecrire/inc/autoriser.php
index e230b8f8e..9f1b055d2 100644
--- a/ecrire/inc/autoriser.php
+++ b/ecrire/inc/autoriser.php
@@ -163,6 +163,9 @@ function autoriser_dist(string $faire, ?string $type = '', $id = null, $qui = nu
 		$qui = array_merge(['statut' => '', 'id_auteur' => 0, 'webmestre' => 'non'], $qui);
 	} elseif (is_numeric($qui)) {
 		$qui = sql_fetsel('*', 'spip_auteurs', 'id_auteur=' . $qui);
+		if (!$qui) {
+			return false;
+		}
 	}

 	// Admins restreints, on construit ici (pas generique mais...)
@@ -1128,7 +1131,7 @@ function autoriser_auteur_modifier_dist(string $faire, string $type, $id, array
 		// ou si les webmestres sont fixes par constante (securite)
 		return false;
 	} // et modifier un webmestre si il ne l'est pas lui meme
-	elseif (intval($id) and autoriser('webmestre', '', 0, $id) and !autoriser('webmestre')) {
+	elseif (!autoriser('webmestre') and intval($id) and autoriser('webmestre', '', 0, $id)) {
 		return false;
 	} else {
 		return true;

Patch possible. On inverse le test dans auteur modifier pour éviter des requêtes SQL inutiles.

Je précise que l’erreur survenait en allant sur la page de controle des urls qui fait une autorisation pour savoir si on peut modifier l’url…
Et si c’était une url d’un auteur inexistant, bah paf…

```diff diff --git a/ecrire/inc/autoriser.php b/ecrire/inc/autoriser.php index e230b8f8e..9f1b055d2 100644 --- a/ecrire/inc/autoriser.php +++ b/ecrire/inc/autoriser.php @@ -163,6 +163,9 @@ function autoriser_dist(string $faire, ?string $type = '', $id = null, $qui = nu $qui = array_merge(['statut' => '', 'id_auteur' => 0, 'webmestre' => 'non'], $qui); } elseif (is_numeric($qui)) { $qui = sql_fetsel('*', 'spip_auteurs', 'id_auteur=' . $qui); + if (!$qui) { + return false; + } } // Admins restreints, on construit ici (pas generique mais...) @@ -1128,7 +1131,7 @@ function autoriser_auteur_modifier_dist(string $faire, string $type, $id, array // ou si les webmestres sont fixes par constante (securite) return false; } // et modifier un webmestre si il ne l'est pas lui meme - elseif (intval($id) and autoriser('webmestre', '', 0, $id) and !autoriser('webmestre')) { + elseif (!autoriser('webmestre') and intval($id) and autoriser('webmestre', '', 0, $id)) { return false; } else { return true; ``` Patch possible. On inverse le test dans auteur modifier pour éviter des requêtes SQL inutiles. Je précise que l’erreur survenait en allant sur la page de controle des urls qui fait une autorisation pour savoir si on peut modifier l’url… Et si c’était une url d’un auteur inexistant, bah paf…
b_b added the
bug
label 3 weeks ago
b_b commented 3 weeks ago
Owner

La 4.0 est aussi concernée ?

La 4.0 est aussi concernée ?
Poster
Owner

Oui et non, y a pas de typage sur les autoriser_* en 4.0.
Du coup ça doit juste faire un beau warning sur if ($qui['statut'] ...
Mais pas de fatale.

Oui et non, y a pas de typage sur les autoriser_* en 4.0. Du coup ça doit juste faire un beau warning sur `if ($qui['statut'] ...` Mais pas de fatale.
b_b added this to the 4.1 milestone 3 weeks ago
cerdic closed this issue 3 weeks ago
Sign in to join this conversation.
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.