Project

General

Profile

Bug #2506

EAP: eap_vendor_type_from_string() sometimes returns wrong result when parsing eap vendor types (eap-254-####)

Added by Reinhard Pfau 11 months ago. Updated 11 months ago.

Status:
Closed
Priority:
Normal
Category:
libstrongswan
Target version:
Start date:
04.01.2018
Due date:
Estimated time:
Affected version:
5.6.1
Resolution:
Fixed

Description

When parsing EAP vendor types "eap-254-####" eap_vendor_type_from_string() sometimes returns a wrong result even when fed with a correct string.

The problem is the test of errno variable after stroul() is called.
strtoul() only sets errno in case of an error; else it is left untouched.

If errno was already != 0 (due to some failure earlier in the current thread) the check fails.
In case of standard EAP types this only leads to a (misleading) debug message ("unknown or invalid EAP method: ###"), but since type var is correctly set it doesn't harm.

But in case of EAP vendor types ("eap-254-<vendor>") it prevents the vendor type to be parsed.
So the result of the func is an EAP-Expanded type with 0 vendor (in case errno was != 0 when the func is called).

We found this in 5.5.2, but the problem exists in 5.6.1 as well.

Solution is simple: reset errno before calling strtoul().
Patch (against 5.6.1) is attached.

Greetings,
Reinhard Pfau, Guntermann & Drunck GmbH

strongSwan-eap-reset-errno-before-using-stroul.patch (1008 Bytes) strongSwan-eap-reset-errno-before-using-stroul.patch patch solving the bug Reinhard Pfau, 04.01.2018 09:39

Associated revisions

Revision a8e940ad (diff)
Added by Reinhard Pfau 11 months ago

eap: Reset errno before calling strtoul() to parse EAP type

Reset errno to 0 before calling strtoul() since it sets errno only on
error cases. So the following test fails even on correct conversions if
errno had a value != 0.

Fixes #2506.

History

#1 Updated by Tobias Brunner 11 months ago

  • Status changed from New to Closed
  • Assignee set to Tobias Brunner
  • Target version set to 5.6.2
  • Resolution set to Fixed

strtoul() only sets errno in case of an error; else it is left untouched.
...
Solution is simple: reset errno before calling strtoul().

Yeah, the NOTES section of the man page even mentions this explicitly. Luckily, this seems to be the only place we use any of the strto* functions this way without setting errno.

Thanks for the patch (please use git format-patch next time), applied to master.

Also available in: Atom PDF