Tom, Mark,

I haven't tried myself, but the code is quite clear:

https://github.com/openvehicles/Open-Vehicle-Monitoring-System/blob/master/vehicle/OVMS.X/net_sms.c#L237

strlen(p) is 0 on an empty REGPHONE, so the result is true. Only "REGISTER" and "AP" need the password, all other commands should succeed from any phone with an empty REGPHONE.

The acc module is not included in the Twizy firmware (due to ROM space).

Securing par_write() is a good idea, but the data loss need not have been caused by a par_write() operation. I know of at least one user module that loses and garbles EEPROM contents frequently. There may be some general hardware issue, at least with the modules delivered during the last 6-9 months or so.

I'm also having single byte EEPROM errors occasionally after flashing. These mostly affect the first byte of some parameter slot, only bit flips up to now but a change to 0 would empty that slot.

If I can confirm the problem I will include both the par_write() securing and the auth fail-close change into the fix.

Regards,
Michael

Am 22.07.2015 um 04:02 schrieb Mark Webb-Johnson:

Michael, Tom,

Not sure how the entire EEPROM can be erased. The code in par_write(), par_set() and par_setbin() seems to limit the write to PARAM_MAX_LENGTH. The EEPROM can’t be overwritten by normal code and it takes quite a bit of register mashing to write to it. Can someone have a look over the params.c functions and see if you have any guesses as to the cause? I think it is important to identify the root cause.

I did fix an error in the acc code, December last year, that caused an off-by-one parameter to be set:

commit 5269c870b1858f58098b16b39d10beb2b0e3b330

Author: Mark Webb-Johnson <mark@webb-johnson.net>

Date: Mon Dec 8 13:11:32 2014 +0800

Setting acc params, without location, when not in a location, leads to off-by-one index parameter write

diff --git a/vehicle/OVMS.X/acc.c b/vehicle/OVMS.X/acc.c

index 4935590..1bf430c 100644

--- a/vehicle/OVMS.X/acc.c

+++ b/vehicle/OVMS.X/acc.c

@@ -948,7 +948,7 @@ BOOL acc_cmd_params(BOOL sms, char* caller, char *arguments)

}

net_send_sms_start(caller);

- if (k<0)

+ if (k<=0)

{

s = stp_rom(net_scratchpad,ACC_NOTHERE);

}

That is in the development branch, but is not in the standard firmware. But, the worst affect of that would be to overwrite PARAM_COOLDOWN (#15) (and easy to see as the overwritten value is base64).

It seems to be that params.c sets the default values to be:

#0 PARAM_REGPHONE = NOPHONE

#1 PARAM_MODULEPASS = OVMS

#2 PARAM_MILESKM = K

My only concern with removing the fail-open to both PARAM_REGPHONE (net_sms_checkcaller) and PARAM_MODULEPASS (net_sms_checkpassarg) is that if they were both blank then the user would be completely locked out of the module. Only solution would be a complete re-flash and reset to default parameters.

Another option would be to hard-code change par_write() to refuse to write a blank password to #1. Then, we could go ahead and change net_sms_checkcaller() to not fail open for registered phone number. From what I can see, all writes to EEPROM go through par_write() in params.c.

Regards, Mark.

On 22 Jul, 2015, at 8:39 am, Tom Saxton <tom@idleloop.com> wrote:

Hi Michael,

Have you confirmed the security problem? I've had time when the EEPROM got
zeroed out and I couldn't access the module via SMS at all. I had to
reflash with a valid EEPROM image in order to re-register my cell number.

I'm pretty careful not to share my OVMS phone number with anyone. If
someone unfriendly had it, they could send it a bunch of SMS messages each
of which costs me a penny. Do that 1,000 times, and they've disabled my
box until I refill my H2O account.

Tom

-----Original Message-----
From: Michael Balzer <dexter@expeedo.de>
Reply-To: OVMS Developers <ovmsdev@lists.teslaclub.hk>
Date: Tuesday, July 21, 2015 at 9:23 AM
To: OVMS Developers <ovmsdev@lists.teslaclub.hk>
Subject: [Ovmsdev] Registered phone / password security issue

Hi everyone,

some users including me experienced loss or garbling of eeprom
parameters. I once saw by chance that my registered phone number had
turned empty -- I seldom need to use SMS, so I only saw this when
checking the parameter list on the App for another config.

The same now also occured to another user I'm in contact with, and it
now turned out that's not only annoying but a security issue:

The current logic of net_sms_checkcaller() allows access to any phone
number if the parameter is empty. The same applies to
net_sms_checkpassarg(), which will allow any password to be used if no
password is stored.

As this kind of data loss can only be detected by checking the
parameters, it's possible to check for "open" modules by just trying to
access them from time to time -- you only need to know the SIM card
number.

I'm about to submit a change for both functions to NOT allow access if
their respective param slots are empty.

As the initial flash contents has the "OVMS" standard password, a
completely lost module should still be restorable by re-flashing.

Do I miss something? Is there any reason for the "empty=wildcard"
behaviour?

Regards,
Michael