Traffic Totals: Broken in 2.4.4-p3 [SOLVED WITH PATCH]

[craibo]

I'm experiencing the same issue when using a non-admin user in all browsers.

The data is all there in the response body but the graph and data table are not rendering.

console failure:

Uncaught TypeError: Cannot read property 'substring' of undefined
    at Object.success (status_traffic_totals.php:475)
    at i (jquery-1.12.0.min.js?v=1538660271:2)
    at Object.fireWith [as resolveWith] (jquery-1.12.0.min.js?v=1538660271:2)
    at y (jquery-1.12.0.min.js?v=1538660271:4)
    at XMLHttpRequest.c (jquery-1.12.0.min.js?v=1538660271:4)

Additionally as previously noted the Hourly Daily Monthly Top 10 Days options are also missing on all users.

[jlw52761]

Interesting, I don't even see a Traffic Totals menu entry under Status, or anywhere for that matter. Running 2.4.4-p3 (upgraded from 2.4.4-p2)

[KOM]

@jlw52761 It's a package that you have to add, Status_Traffic_Totals.

[jlw52761]

@KOM Got it, thanks!

[jimp]

Which specific privileges do the user have which can't load the graph?

The WebCfg - Status: Traffic Totals (page-status-monitoring) privilege appears to be correct.

What, if anything, shows in the main system log when a non-admin user attempts to access the page?

[NogBadTheBad]

@jimp said in Traffic Totals: Broken in 2.4.4-p3 [Cause Identified]:

Which specific privileges do the user have which can't load the graph?

The WebCfg - Status: Traffic Totals (page-status-monitoring) privilege appears to be correct.

What, if anything, shows in the main system log when a non-admin user attempts to access the page?

I only have the single user.

From the FreeRadius users:-

"andy" Cleartext-Password := "password"

	Class := "admins",
	Service-Type := "Administrative-User"

[jimp]

Actually, I see what the problem is. The way the package uses display_top_tabs() to generate tabs that don't link to actual pages, just JS anchors, doesn't like the new stronger page validation used by the privilege system. And since they aren't actual files that exist, there isn't a way to allow access to them, so the privilege system filters out the tabs.

I don't see a quick way to fix this in the package privileges, but maybe the package maintainer can figure out a better way to generate the tab anchor links.

I'll see if I can come up with a safe way to test for this in the privilege matching system.

[NogBadTheBad]

Thanks Jim

[jimp]

https://redmine.pfsense.org/issues/9550

[jimp]

I think I've got this fixed but it'll take a patch in the base system, not the package.

You can install the System Patches package and then create an entry for bdbd8534eef5b93370065340de225a1cd5e5faa8 to apply the fix and try it out. I did test against several different attack methods to ensure it didn't lower the security, and it allows the JS anchor links as expected.

[Nyarlathotep]

@jimp

Thanks for the patch; it fixed the issue. I also applied the User Manager bug patch.

I was prompted to install the User Manager patch after seeing Tom's latest video.

[fabrizior]

@jimp said in Traffic Totals: Broken in 2.4.4-p3 [SOLVED WITH PATCH]:

I think I've got this fixed but it'll take a patch in the base system, not the package.

You can install the System Patches package and then create an entry for bdbd8534eef5b93370065340de225a1cd5e5faa8 to apply the fix and try it out. I did test against several different attack methods to ensure it didn't lower the security, and it allows the JS anchor links as expected.

@jimp Thanks for this. One question though. Testing the patch indicates it cannot be backed-out cleanly. Is this something we should be concerned about?

/usr/bin/patch --directory=/ -f -p2 -i /var/patches/5cebf5d50a1d0.patch --check --reverse --ignore-whitespace

Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|From bdbd8534eef5b93370065340de225a1cd5e5faa8 Mon Sep 17 00:00:00 2001
|From: jim-p 
|Date: Fri, 24 May 2019 15:47:43 -0400
|Subject: [PATCH] Privilege matching -- allow JS anchors. Fixes #9550
|
|Attempts to detect a special case where a file does not actually
|exist, and yet should be allowed since it is used by JavaScript.
|
|So long as the anchor name doesn't contain any characters that might let
|it evade other checks, allow it through.
|---
| src/etc/inc/auth_func.inc | 10 ++++++++++
| 1 file changed, 10 insertions(+)
|
|diff --git a/src/etc/inc/auth_func.inc b/src/etc/inc/auth_func.inc
|index 795ccdbdf1..e142e4f42c 100644
|--- a/src/etc/inc/auth_func.inc
|+++ b/src/etc/inc/auth_func.inc
--------------------------
Patching file etc/inc/auth_func.inc using Plan A...
Hunk #1 failed at 42.
1 out of 1 hunks failed while patching etc/inc/auth_func.inc
done

[Derelict]

When you test a patch it shows its current status when compared to the file(s) to be patched.

Before you apply it will say:

Patch can be applied cleanly (detail)
Patch can NOT be reverted cleanly (detail)

After it is applied it will say:

Patch can NOT be applied cleanly (detail)
Patch can be reverted cleanly (detail)

[fabrizior]

@Derelict, ah. okay then.
Thank you.